GNOME Bugzilla – Bug 568555
jpegdec: Interlaced MJPEG files not handled correctly
Last modified: 2017-09-05 11:47:15 UTC
For example: http://gstreamer.freedesktop.org/media/incoming/douche.MJPEG-A.mov With this kind of file, each packet from qtdemux contains 2 sequential jpeg images, one per field. jpegdec detects that it is running in 'packetised' mode, and only decodes the first jpeg image in each packet, resulting in half-height output of online the first field. It needs to allocate a buffer twice the height and then decode each field into that buffer. I'm not sure what happens with the sub-sampled chroma planes in this case. I guess each field contains a subsampled version of that field only. ffdec_mjpeg correctly decodes both jpeg images in the packet, and outputs a Y42B frame.
Also http://samples.mplayerhq.hu/avi/TRA3106.avi
Created attachment 198317 [details] [review] jpegdec: properly decode interlaced mjpeg In packetized mode, detect multiple consecutive jpeg images and decode them all. Tested with one stream with two images per buffer in YUV colorspace. Other colorspaces handled, but untested. More than two images per field should work, but also untested.
Tested with the first sample, but not the second, which is 403.
Created attachment 198326 [details] [review] jpegdec: properly decode interlaced mjpeg In packetized mode, detect multiple consecutive jpeg images and decode them all. Tested with one stream with two images per buffer in YUV colorspace. Other colorspaces handled, but untested. More than two images per field should work, but also untested.
Wouldn't this be better fixed in jpegparse by properly parsing the consecutive jpeg images into different buffers? Or am I missing something here?
Possibly... You'd end up with half height images. Maybe deinterlace might work on those. Seems a bit iffy to me, but then all the deinterlace stuff always seemed iffy to me.
I think it needs to be done in jpegdec as well, we don't do fields in separate buffers so far and I don't think we should start now. Besides, I hope jpegparse isn't autoplugged yet, that'd be a bit scary. For what it's worth, I have this implemented as well locally, but never got around to fixing the rebase merge conflicts with current master ;) Anyway, I'll try to have a look at this later or in the next few days to compare the approaches. Mine was a bit different (without the parsing).
Oh, this is only for the different fields. Sorry, ignore my comment :)
Any of you wants to chime in on whether it's fine to push this before the freeze ?
Sorry, it's still high on my todo list. There isn't really a freeze this time anyway (if there was, we'd be already frozen), but I'd also like to get this in if possible.
Cool, you wouldn't happen to have that mplayer sample lying around by any chance ? It's still 403 here, so I could test only with the douche sample. If not, I'll push the patch soonish.
I've fixed up the stuff I had and pushed it here: http://cgit.freedesktop.org/~tpm/gst-plugins-good/ (might still be missing one or two commits, not sure). The other sample file is here: http://people.freedesktop.org/~tpm/samples/564100-mjpeg-premature-eos-TRA3106.avi
Thanks for the file. Works for my patch. Yours appears to include cleanup so is probably the best one to push.
This bug has an assigned developer but has not received activity in almost a year. Is the assigned person still working on this ?
Kinda. Needs updating for new base class.
Tim, Vincent? Anybody planning to rebase the patch and get it in?
It needs more than just rebasing, but yes, I'm planning to look at it soon.
commit 09ee89f4c4ccbb1de21f7b7ed38dcc2d44f4253c (HEAD -> master) Author: Sebastian Dröge <sebastian@centricular.com> Date: Tue Sep 5 13:28:16 2017 +0300 jpegdec: Handle interlaced MJPEG streams These come with two JPEG images per buffer of half height than signalled in the container. Changes based on Tim-Philipp Müller's 0.10 branch: https://cgit.freedesktop.org/~tpm/gst-plugins-good/log/?h=jpegdec-interlaced https://bugzilla.gnome.org/show_bug.cgi?id=568555