GNOME Bugzilla – Bug 774112
mpdparser: MS PlayReady ContentProtection event is truncated
Last modified: 2016-11-14 11:11:40 UTC
When the demuxer emits the protection event it uses strlen() to get the data length but in some cases (PlayReady...) the data can have a \0 before the end of the actual data. The proposed fix is to store the base64 decoded_len in the descriptor and fallback to strlen() in other cases.
If it's not a string, don't store it as a string. Use a GstBuffer instead for example. As everybody will need it base64 decoded, it probably makes sense to do that in adaptivedemux already.
Not sure what you mean about doing this in adaptivedemux, using a new dedicated vfunc? I see that mssdemux indeed already wraps the base64 decoded data in a buffer :)
I mean doing base64 decoding here (once instead of in every user of the event, or is base64 encoding part of the event API?), and storing things in a GBytes or GstBuffer if it's not a string (i.e. can contain \0 and other things).
From what I could observe so far, PlayReady seems to be the only system encoding its data with base64. This isn't part of the event API, protection events use a GstBuffer already. Currently the demuxers use gst_adaptive_demux_stream_queue_event() to handle the protection events they create. We could have adaptivedemux base64-decode events buffers if the system_id matches PlayReady. Would that be ok?
Ok in that case maybe just leave it as given by the stream, leave it base64 and put it into a GstBuffer. \0 in there would be something for the consumer to worry about then (which might require it base64 encoded anyway?)
So the consumer of the event would be responsible to decode the buffer data? I'm fine with that but it would be worth a paragraph in the release notes :)
(In reply to Philippe Normand from comment #6) > So the consumer of the event would be responsible to decode the buffer data? > > I'm fine with that but it would be worth a paragraph in the release notes :) There was no support for this in 1.10, it's completely new, right?
(In reply to Sebastian Dröge (slomo) from comment #7) > (In reply to Philippe Normand from comment #6) > > So the consumer of the event would be responsible to decode the buffer data? > > > > I'm fine with that but it would be worth a paragraph in the release notes :) > > There was no support for this in 1.10, it's completely new, right? mssdemux in the 1.8 branch has this base64-decoding support: https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/ext/smoothstreaming/gstmssdemux.c?h=1.8#n476 Same in 1.10 :) So this would be a change of behavior for 1.12.
Created attachment 339786 [details] [review] mssdemux: wrap unmodified data in protection event The base64 decoding operation now needs to be done by the protection event consumer.
Created attachment 339787 [details] [review] mpdparser: wrap unmodified data in protection event The base64 decoding operation now needs to be done by the protection event consumer.
Ok, please also provide a patch as reminder for the www module then (there's already an empty 1.12 release notes files). Can be just a bullet point somewhere
Comment on attachment 339786 [details] [review] mssdemux: wrap unmodified data in protection event Maybe mention in the commit message *why* :)
Comment on attachment 339786 [details] [review] mssdemux: wrap unmodified data in protection event Pushed as b35979c31aa1f812a1d3cfdeb5b4e7188f97bb7e with commit message amended.
Comment on attachment 339787 [details] [review] mpdparser: wrap unmodified data in protection event Pushed as 0fbd2edaffe97e920aa0fa8a509256a1a70a3bfe with commit message amended and unit-test updated.
Created attachment 339790 [details] [review] release-notes-1.12: document Protection buffer storage changes
Comment on attachment 339790 [details] [review] release-notes-1.12: document Protection buffer storage changes Thanks, we can move that to a better section later if needed (e.g. if there's a adaptive streaming section as usual), or shorten it, or whatever is needed later :)