Add javascript signalling

[aiden-jeffrey]

Based on this sample, this adds a way to signal in javascript (from the webpage) a couple of things:

• "ready" - page is ready for rendering, so enable cefsrc to go to a PAUSED/PLAYING state
• "eos" - page doesn't have any more content, so push an EOS event onto cefsrc

This is useful for pages that have non-trivial load times and may run out of content (webgl pages, pages with videos, etc).

There is a sample in the html/ directory that shows how to send the signals, e.g.:

window.gstSendMsg({
          request: "ready",
          onSuccess: function(response) { ... }
          },
          onFailure: function(error_code, error_message) { ... }
        });
      }

To test, run a webserver in the html directory, say:

python -m http.server 9000

Then just specify the flag in a run pointing at that page:

gst-launch-1.0 -e cefsrc url="http://localhost:9000/ready_test.html" listen-for-js-signals=true ! video/x-raw, width=1920, height=1080, framerate=60/1 ! cefdemux name=d d.video ! queue max-size-bytes=0 max-size-buffers=0 max-size-time=3000000000 ! videoconvert ! xvimagesink

[MathieuDuponchelle]

Whee, that looks very interesting, I'll review as soon as I find time :)

[MathieuDuponchelle]

Can you expand on the commit message for "message: Add message routers and handlers ", explaining why this is actually needed? The code looks fine but I can't quite understand the intent behind it :)

[MathieuDuponchelle]

I know this wasn't the case before, but I think you will want to also add the posting of progress messages to gst_cef_src_start() (refer to eg rtspsrc for an example).

The start method was already blocking while waiting for cef initialization, which isn't great without posting progress messages, but if we now also (optionally) wait for an arbitrary amount of time for the web page to reach a certain state it becomes even more critical to post these :)

You should notice the difference in behavior with gst-launch after the messages are added.

[MathieuDuponchelle]

Sorry for not reviewing earlier ! This looks very useful to me, though admittedly not entirely generic, I'm OK with merging once my comments are addressed :)

[aiden-jeffrey]

though admittedly not entirely generic

I toyed with allowing the user to send all gstreamer events from javascript, but then I'm not sure there is an event that correlates to the "ready" signal I add here. If you can think of a way of making this more flexible / generic, then I'd be happy to improve the API.

[MathieuDuponchelle]

If you can think of a way of making this more flexible / generic, then I'd be happy to improve the API.

No, I think that's fine for now :)

[aiden-jeffrey]

I know this wasn't the case before, but I think you will want to also add the posting of progress messages to gst_cef_src_start() (refer to eg rtspsrc for an example).

Is there a reason cef_status is file level static and not in the GstCefSrc struct?

[MathieuDuponchelle]

Is there a reason cef_status is file level static and not in the GstCefSrc struct?

Oh hrm wait, this question makes me realize you're probably piggy-backing the wrong mechanism :)

[MathieuDuponchelle]

cef status is global, it tracks whether cef needs to be deinitialized (because all browsers / cef sources are done rendering).

[aiden-jeffrey]

Oh hrm wait, this question makes me realize you're probably piggy-backing the wrong mechanism :)

Yeah.

[MathieuDuponchelle]

Here you want a separate, per source additional mechanism to wait for readiness, you can probably mirror the existing one

[aiden-jeffrey]

Should I use the state_lock and state_cond for the wait mechanics?

[MathieuDuponchelle]

Yes, that seems appropriate

[MathieuDuponchelle]

Can you squash the following commits together:

signals: Add support for starting and stopping cefsrc from js

signals: Move wait-ready mutex into src struct

?

Re progress messages, that is not what I meant, here is an example:

https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.c?ref_type=heads#L6319-6375

[aiden-jeffrey]

Re progress messages, that is not what I meant

So replace where I've added GST_DEBUG_OBJECT calls with your GST_ELEMENT_PROGRESS macro?

[SteveMcFarlin]

btw. Thanks for the patch.

[aiden-jeffrey]

I'm getting a seg fault when starting another pipeline with a cefsrc in after the first cefsrc has emmited the eos due to the javascript signal. It segfaults somewhere in CefInitialize, but i can't work out where because I can't get gstreamer to pick up the plugin when built with Debug symbols.

[aiden-jeffrey]

I'm getting a seg fault

Managed to trick the loader to load the debug version - this is what's failing:

[1881725:1889451:1002/150401.084126:FATAL:startup_metric_utils.cc(54)] Check failed: application_start_ticks_.is_null(). 
Trace/breakpoint trap (core dumped)

@MathieuDuponchelle - do you have any ideas? It looks like I'm not shutting things down properly (the cef_status goes to SHUTDOWN after the eos, so I'm not sure where the cleanup is broken).

[MathieuDuponchelle]

ouch, I don't have an idea right now no

[MathieuDuponchelle]

can you post a test case?

[aiden-jeffrey]

can you post a test case?

I don't have a gst-launch command that allows for this behaviour. Let me see if I can make a simplified version of our application to replicate.

But the basic idea is:

• create two pipelines
• start one
• wait for the eos triggered via the js signal
• watch the cef_status go to 0
• start second one
• see CefInitialize call segfault

[MathieuDuponchelle]

OK, yes please make a reproducer application so I can take a look on my end, otherwise I'm afraid we'll have to revert this until we come with a fix for the issue :)

[aiden-jeffrey]

@MathieuDuponchelle - my colleague set up a repro example last night - I think this is a bug that pre-existed this signals work. See https://github.com/aiden-jeffrey/test-cefsrc for the sample. There's two in there. I managed to repro the segfault on the sha prior to the signals work (002f660) using the sleep binary.

[aiden-jeffrey]

It's possible I need to do more cleanup on the rust side. I'm not super confident in rust.

[aiden-jeffrey]

@MathieuDuponchelle - I think it might be a regression. 5643c02bda8a67f9397aa15d4905a52c00d6cc16 doesn't have this crash.

I performed a bisect between 5643c02 and 002f660, I had to skip some commits, this was the result of my bisect:

There are only 'skip'ped commits left to test.
The first bad commit could be any of:
189f7bcca308dd0a2f93e8090ac3b153cd542bce
bf0c9420aac6d58446e5638b7803c8e0ef21e034
4e79b292a6eff3f1179b24329fc263bb84e979c5
bc511024498892cb271da756833c8c4fb674dbc2
6e469dc064cef5d78431285784fc20b0f2108a15
779761f653a7d6d95344699aa534723c95ab7a4e
1e4f194c5546bb1fa70e139d28277e9cfcc6f1cd
fb9be4f9e5a88c7ca3880ce0edce7dd7ec51de65
4b8a7447e154ef16327f4bcc3c38a01ef6c31f9e
8383a4c3a8012d9919aa7e1000807c2671b06fef
187b280ddadf388d29b83d57331d83602a193707
bfa3de0fdbcbc22d26e29076595c1ff7742e3b64
6f1823cf71234c7fd19261f53bb6754cadc7e380
We cannot bisect more!

[MathieuDuponchelle]

@aiden-jeffrey yeah I was wondering if this was a regression from #83 , it looks likely that it is related. Why did you need to skip these commits? cc @amyspark

[MathieuDuponchelle]

Thanks for the reproducer by the way

[aiden-jeffrey]

Why did you need to skip these commits?

I got some unrelated errors trying to run the sleep binary.

For half of these (for example 189f7bc) I got:

$ cargo run --bin sleep
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
     Running `target/debug/sleep`

(gst-plugin-scanner:1632010): GLib-GObject-CRITICAL **: 13:33:35.045: g_param_spec_enum: assertion 'g_enum_get_value (enum_class, default_value) != NULL' failed

(gst-plugin-scanner:1632010): GLib-GObject-CRITICAL **: 13:33:35.045: validate_pspec_to_install: assertion 'G_IS_PARAM_SPEC (pspec)' failed

(gst-plugin-scanner:1632010): GLib-GObject-CRITICAL **: 13:33:35.045: g_param_spec_ref_sink: assertion 'G_IS_PARAM_SPEC (pspec)' failed

(gst-plugin-scanner:1632010): GLib-GObject-CRITICAL **: 13:33:35.045: g_param_spec_unref: assertion 'G_IS_PARAM_SPEC (pspec)' failed

(sleep:1632002): GLib-GObject-CRITICAL **: 13:33:35.098: g_param_spec_enum: assertion 'g_enum_get_value (enum_class, default_value) != NULL' failed

(sleep:1632002): GLib-GObject-CRITICAL **: 13:33:35.098: validate_pspec_to_install: assertion 'G_IS_PARAM_SPEC (pspec)' failed

(sleep:1632002): GLib-GObject-CRITICAL **: 13:33:35.098: g_param_spec_ref_sink: assertion 'G_IS_PARAM_SPEC (pspec)' failed

(sleep:1632002): GLib-GObject-CRITICAL **: 13:33:35.098: g_param_spec_unref: assertion 'G_IS_PARAM_SPEC (pspec)' failed

• for a few (e.g. bfa3de0) I got:

GStreamer-WARNING **: 13:36:37.263: Failed to load plugin '/home/aiden/-/gstcefsrc/build/Release/libgstcef.so': /home/aiden/-/gstcefsrc/build/Release/libgstcef.so: undefined symbol: cef_string_multimap_free

[MathieuDuponchelle]

I see, annoying that the history doesn't build cleanly

[aiden-jeffrey]

yeah I was wondering if this was a regression from #83 ,

I tried reverting this merge commit and the issue still persists.

[aiden-jeffrey]

I tried reverting this merge commit and the issue still persists.

See https://github.com/aiden-jeffrey/gstcefsrc/tree/aiden/try-revert.

[MathieuDuponchelle]

Hm, there was no merge commit, but 14 commits merged fast forward, I'm currently building the branch right before the first one of those

[MathieuDuponchelle]

That's #86 , not #83 :)

[aiden-jeffrey]

Just spotted my mistake! Doh.

[MathieuDuponchelle]

yeah, some commit in #83 introduced the issue, @amyspark do you mind taking a look? cargo run --bin sleep in https://github.com/aiden-jeffrey/test-cefsrc segfaults with master, completes successfully with d33494c and prior :)

[aiden-jeffrey]

Pushed a small change (removing log-severity) to that test repo. Now my bisect completes and identifies bc511024498892cb271da756833c8c4fb674dbc2 as the first bad commit.

[aiden-jeffrey]

I imagine it relates to cef_inited.

[MathieuDuponchelle]

I imagine it relates to cef_inited.

In some fashion for sure :)

[aiden-jeffrey]

I think the issue here is CefShutdown is getting called, which according to the header should only be called on application exit. I've removed that, but now can't get the MakeBrowser CefTask to run in the second pipeline.

[aiden-jeffrey]

Ok. Multiple pipelines just aren't really supported the way this element is currently written. If you run two cefsrc elements at the same time (without calling CefShutdown), Initialize never gets acalled, which means that the second cefsrc just has the properties of the first.

[MathieuDuponchelle]

@aiden-jeffrey CefInitialize being called a single time is not a problem, but yes the javascript flags property is only used for the first source, which isn't great, other properties don't affect CefInitialize afaict

[MathieuDuponchelle]

Ah, sandbox too, perhaps others, anyway see #90 , CefInitialize simply can't be called more than once in a given process