After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 793551 - vc1parse: parsing issues with vc1 streams encoded in non-WMV containers
vc1parse: parsing issues with vc1 streams encoded in non-WMV containers
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.13.x
Other Windows
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-02-17 21:54 UTC by Ishmael Visayana Sameen
Modified: 2018-11-03 14:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
vc1parser: Relax ASF Binding Byte validation (1.34 KB, patch)
2018-02-18 00:40 UTC, Nicolas Dufresne (ndufresne)
none Details | Review

Description Ishmael Visayana Sameen 2018-02-17 21:54:43 UTC
Currently, msdk / mfx VC1 decoder plugins require vc1parse to work properly with VC1 streams. There are issues however when dealing with VC1 streams encoded in non-WMV containers such as Matroska (.mkv) or MP4, for e.g., executing the simple pipeline using a sample clip (https://www.techpowerup.com/download/hd-dvd-demo-1080p-vc-1-ddplus-5-1/):

gst-launch-1.0 filesrc location=/path/to/hddvd_demo_1080p.mkv ! matroskademux ! vc1parse ! fakesink

This bug was previously reported in https://github.com/intel/gstreamer-media-SDK/issues/65 but the solution employed is hack-ish.
Comment 1 Nicolas Dufresne (ndufresne) 2018-02-17 23:51:59 UTC
Interestingly, you can replace vc1parse with avdec_vc1 and it decodes it. Looking at that decoder, it seems to pretty much ignore the codec_data (extradata). So it seems a little redundant somehow.

Meanwhile, looking at the generated data, with my very limited knowledge of VC1 headers forma you can see that the code_data from matroska starts with:

  240000010f...

So 1 byte set to 0x24, followed by a start code with 0x0F, VC1_SEQUENCE. And later one we can see another startcode with 0x0E, VC1_ENTRYPOINT. So the only thing that blocks the parser is that is currently expect the two first bytes to be 0x01, which can be demonstrated with the following pipeline:

  gst-launch-1.0 -v filesrc location=hddvd_demo_1080p.mkv ! matroskademux ! capssetter caps=video/x-wmv,codec_data=\(buffer\)010000010fdbfe3bf21bca3bf886f180ca02020309a5b8d707fc0000010e5ac7fcefc86c40 ! vc1parse ! fakesink

If someone can point me to what defines these numbers, 0x01 and 0x24, writing a patch to supports this (or fixing matroskademux maybe ?) seems rather trivial.
Comment 2 Nicolas Dufresne (ndufresne) 2018-02-18 00:09:19 UTC
Small correction, the parser expect the least significant bit (of the first byte) to be 1. So 25 would work, where 24 fails. I really need to know what this is about and why we do this validation.
Comment 3 Nicolas Dufresne (ndufresne) 2018-02-18 00:29:24 UTC
Ok, found it. So this has nothing to do with Matroska, it has to do with your broken encoder/muxer. Clearly you are setting a codec data defined as per the "A.5 VC-1 stream headers in CodecPrivateData". Matroska does not redefine this. This byte is defined by "A.6 ASF binding byte for Advanced Profile in ASF CodecPrivateData."

According to the spec, the least significant bit is reserved and should always be 1. So this CODEC data is a violation of the spec. For the rest, if we consider the rest, we have:

  Bit 1 = 0 :: b frame might be present
  Bit 2 = 1 :: there no slice
  Bit 3 = 0 :: multiple entry headers might be present
  Bit 4 = 0 :: multiple sequence headers might be present
  Bit 5 = 1 :: there is no interlaced frames
  Bit 6 = 0 :: reserved no default
  Bit 7 = 0 :: reserved no default

Now, if FFMPEG accepts it, I think we should also do. So I'll relax the validation a bit. It will still be safe really. The remaining question is if it's vc1parser job to actually fix it. This way, we don't maintain this bug in transmuxing. It's a very minor issue in the bitstream to be honest.
Comment 4 Nicolas Dufresne (ndufresne) 2018-02-18 00:40:30 UTC
Created attachment 368508 [details] [review]
vc1parser: Relax ASF Binding Byte validation

According to the spec, the least significant bit is reservered and should
always we set to 1. Though, some wrong file has been found. Considering
how low important this reserved bit is, relax the validation.
Comment 5 Nicolas Dufresne (ndufresne) 2018-02-18 00:44:06 UTC
Note that this does not yet make this file play well. I see couple of bugs:
  1. Our AC3 parser refuses to negotiated with matroskamux
  2. Matroskademux refused to only play video
  3.  GStreamer-CRITICAL **: _gst_util_uint64_scale: assertion 'denom != 0' failed (bad framerate=0/1 handling)
  4. Timestamp are going backward, we don't yet implement the TS delta for B-Frames streams
Comment 6 GStreamer system administrator 2018-11-03 14:18:34 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/660.