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 736871 - codecparsers_vc1: sequence-layer parser is broken due to endianness issue.
codecparsers_vc1: sequence-layer parser is broken due to endianness issue.
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal major
: 1.4.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-18 10:20 UTC by Aurélien Zanelli
Modified: 2014-09-19 06:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
codecparsers_vc1: take care of endianness when parsing sequence-layer (3.94 KB, patch)
2014-09-18 10:22 UTC, Aurélien Zanelli
committed Details | Review
codecparsers_vc1: add unit test for sequence-layer parsing (3.36 KB, patch)
2014-09-18 10:23 UTC, Aurélien Zanelli
committed Details | Review

Description Aurélien Zanelli 2014-09-18 10:20:22 UTC
Current sequence-layer vc1 parser is broken due to endianmness issue.

Sequence-layer is serialized in little-endian byte-order except for STRUCT_C.
STRUCT_A and STRUCT_B are therefore serialized in little-endian byte-order but their fields are defined as unsigned int msb first, so likely big-endian.
Comment 1 Aurélien Zanelli 2014-09-18 10:22:08 UTC
Created attachment 286463 [details] [review]
codecparsers_vc1: take care of endianness when parsing sequence-layer

This patch proposal use a GstByteReader instead of a GstBitReader to parse the sequence-layer.

But since STRUCT_A and STRUCT_B fields are definied as unsigned integer msb first, I think we can't change struct_a and struct_b parsing, so I use two temporary structA and structB byte arrays and pass them to parsing functions.

I'm not sure it's the best way to do this, so if you have some other idea, you're welcome. :)
Comment 2 Aurélien Zanelli 2014-09-18 10:23:54 UTC
Created attachment 286464 [details] [review]
codecparsers_vc1: add unit test for sequence-layer parsing 

I also write a unit test for sequence-layer parsing.

It obviously fail with the current vc1parser.
Comment 3 Sebastian Dröge (slomo) 2014-09-18 10:37:33 UTC
It's not beautiful but I can't think of a more beautiful way of doing that :)

(also wtf!)

commit bd050201babf995c16181d4ef3a3bd46c261b8be
Author: Aurélien Zanelli <aurelien.zanelli@parrot.com>
Date:   Thu Sep 18 11:39:53 2014 +0200

    vc1parser: add unit test for sequence-layer parsing
    
    Check that a sequence-layer header is successfully parsed.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=736871

commit c9a196d54dd41b68473e5a15d17cc37713735147
Author: Aurélien Zanelli <aurelien.zanelli@parrot.com>
Date:   Thu Sep 18 11:49:13 2014 +0200

    vc1parser: take care of endianness when parsing sequence-layer
    
    sequence-layer is serialized in little-endian byte order except for
    STRUCT_C which is serialized in big-endian byte order.
    
    But since STRUCT_A and STRUCT_B fields are defined as unsigned int msb
    first, we have to pass them as big-endian to their parsing function. So
    we basically use temporary buffers to convert them in big-endian.
    
    See SMPTE 421M Annex J and L.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=736871
Comment 4 Aurélien Zanelli 2014-09-18 12:14:10 UTC
(In reply to comment #3)
> It's not beautiful but I can't think of a more beautiful way of doing that :)
> 
> (also wtf!)

I think it's a good example of "defective by design" :-P