GNOME Bugzilla – Bug 762516
qtdemux: cenc auxiliary info parsing crashes
Last modified: 2016-04-14 17:42:55 UTC
+++ This bug was initially created as a clone of Bug #755614 +++ +++ This bug was initially created as a clone of Bug #705991 +++ https://github.com/youtube/js_mse_eme/blob/master/media/oops_cenc-20121114-145-no-clear-start.mp4 Crashes when played as a local file but not over HTTP: Program received signal SIGSEGV, Segmentation fault.
+ Trace 235985
Thread 140737202661120 (LWP 745)
I'm preparing a patch for this issue :) The problem is that the cenc metadata is stored outside of the moof box and we try to push a buffer before the metadata has been parsed.
Created attachment 322211 [details] [review] qtdemux: don't push encrypted buffer without cenc metadata When the cenc metadata is stored outside of the moof box and the stream is exposed it is possible that the cenc metadata hasn't been processed yet while the first buffer is being pushed. When this happens the buffer can't possibly be decrypted downstream so don't push it.
Shouldn't the cenc metadata come in any case before the first actual data that has to be decrypted? And if not, would it make sense to queue up buffers until it is available?
(In reply to Sebastian Dröge (slomo) from comment #3) > Shouldn't the cenc metadata come in any case before the first actual data > that has to be decrypted? And if not, would it make sense to queue up > buffers until it is available? When the file is accessed through HTTP the metadata is parsed before the first buffer has to be pushed. But this is not the case if the file is local. Perhaps this is related with how the src element operates? I wonder :) To answer your question, yeah, it might be good indeed to queue buffers until the metadata is parsed, I'll revise the patch.
That sounds like a problem. So you're saying this only happens in PULL mode? Seems like it does things wrong then :) Especially in PULL mode it should be able to collect all headers before the buffers.
Seems so yes, if I use pushfilesrc instead of filesrc it doesn't crash.
I wonder why we try to push a buffer anyway, even after 4 errors were raised: 0:00:00.292522760 983 0x75e01c00 ERROR qtdemux qtdemux.c:7212:gst_qtdemux_configure_protected_caps:<qtdemux0> stream is protected, but no suitable decryptor element has been found 0:00:00.292721510 983 0x75e01c00 ERROR qtdemux qtdemux.c:7364:gst_qtdemux_configure_stream:<qtdemux0> Failed to configure protected stream caps. ... 0:00:00.297545573 983 0x75e01c00 ERROR qtdemux qtdemux.c:7212:gst_qtdemux_configure_protected_caps:<qtdemux0> stream is protected, but no suitable decryptor element has been found 0:00:00.297735625 983 0x75e01c00 ERROR qtdemux qtdemux.c:7364:gst_qtdemux_configure_stream:<qtdemux0> Failed to configure protected stream caps. Shouldn't the demuxer abort if no decryptor was found? In that case the demuxer is pretty much unlinked anyway.
I think it should just fail then, yes. But that's independent of this bug, or not?
Yes. So I think we need two patches: - one checking for NULL crypto_info pointer to prevent any possible crash in this part of the code. Maybe we can assert in this case? - another change making qtdemux_expose_streams() return GST_FLOW_ERROR if any of the streams couldn't be added.
Created attachment 322224 [details] [review] qtdemux: prevent buffer flow if any stream failed to be exposed In some cases the stream configuration can fail, for instance if the stream is protected and no decryptor was found. For those situations the demuxer shouldn't emit any data on the corresponding source pad of the stream and bail out.
And understanding why it does that (reading buffers before reading the headers) in PULL mode but not PUSH mode?
That's going to take time for me. Debugging qtdemux is not so much fun, but I'll compare the logs between both modes, anyway :)
Ok I think I understand this a bit better now. Part of the solution would be to parse the cenc data from the loop_state_movie() function (PULL mode), like it's done in process_adapter() in PUSH mode.
Can you provide a patch for that too?
Yes, I hope so :)
Great, thanks for looking into this :)
Created attachment 322348 [details] [review] qtdemux: cenc aux info parsing from mdat support in PULL mode This is already supported for PUSH mode but was failing in PULL mode. The aux info is sometimes stored in the mdat before the first sample, so the loop task needs to pull data stored at that location and perform the aux info cenc parsing. https://bugzilla.gnome.org/show_bug.cgi?id=761700
Attachment 322211 [details] pushed as fb5d50c - qtdemux: don't push encrypted buffer without cenc metadata Attachment 322224 [details] pushed as 67f3fc1 - qtdemux: prevent buffer flow if any stream failed to be exposed Attachment 322348 [details] pushed as 9c47c0d - qtdemux: cenc aux info parsing from mdat support in PULL mode