GNOME Bugzilla – Bug 366155
[matroskademux] Several problems in encoding handling code
Last modified: 2006-10-30 16:39:54 UTC
There are some problems with the recent patch that adds support for track encoding In gst_matroska_decode_buffer it is assumed that encoding type is compression. And therefore possibly uninitialized value enc.comp_algo is used. Matroska demuxer therefore tries to unzip encrypted tracks. Not all possible values of comp_algo are checked (comp_algo == 3 is missing), therefore new_data may stay NULL and buffer referencing NULL data may be pushed to sink pad. There is a for-cycle over all encodings, but new_data is generated allways from the original buffer (and old new_data leaked). Data should be decoded according to encoding order (I have sorted encodings decreasingly in enc.order in my patch) I have also removed \n from GST_WARNINGS (patch follows)
Created attachment 75556 [details] [review] Fix several decoding problems In this patch, I do not print any warning that data decryption is not supported. In fact, EncodingType == 1 does not imply that the data are encrypted and can not be played as such. Also, I do not like printing warning for every buffer. It would be probably nicer to write the decryption warning only once. I would gladly provide a patch for this if you agree that it is a good idea.
Looks good, thanks. 2006-10-30 Tim-Philipp Müller <tim at centricular dot net> Patch by: Michal Benes <michal dot benes at itonis tv> * gst/matroska/matroska-demux.c: (gst_matroska_demux_encoding_cmp), (gst_matroska_demux_read_track_encodings), (gst_matroska_decode_buffer): Fix several issues with encoded/compressed/encrypted/signed tracks; also, remove superfluous newline characters from some debug statements. (#366155) I'm not entirely sure what to do about buffers we can't decode or decompress. I am not really sure whether it's a good idea to push them as they are (as opposed to just dropping them). We might also want some check in place to ensure that there is at least one track we can decode and throw an error if not. It seems that according to the spec EncodingType == 1 && ContentEncAlgo == 0 means that the buffers are signed rather than encrypted. Is this what you meant or are there more cases? Not printing the warning for every buffer would indeed be a good idea. Care to make a patch for that too? :) Btw, if you happen to have some (not so big) sample files that use track encodings/compressions/encryptions, I'd be quite interested to get my hands on them.