matroskadec: fix endianness fourcc and codec_tag

Message ID 20170301153029.9584-2-stebbins@jetheaddev.com
State New
Headers show

Commit Message

John Stebbins March 1, 2017, 3:30 p.m.
This fixes decode of rawvideo in matroska.
---
 libavformat/matroskadec.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Luca Barbato March 1, 2017, 5:38 p.m. | #1
On 01/03/2017 16:30, John Stebbins wrote:
> This fixes decode of rawvideo in matroska.
> ---
>  libavformat/matroskadec.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 4e121b6..75cfa85 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1612,6 +1612,7 @@ static int matroska_parse_tracks(AVFormatContext *s)
>                  track->video.display_width = track->video.pixel_width;
>              if (!track->video.display_height)
>                  track->video.display_height = track->video.pixel_height;
> +            track->video.fourcc = av_bswap32(track->video.fourcc);
>          } else if (track->type == MATROSKA_TRACK_TYPE_AUDIO) {
>              if (!track->audio.out_samplerate)
>                  track->audio.out_samplerate = track->audio.samplerate;
> 

Would be nice to have some more information in the commit message.

Are you sure you want to swap it even when you are on big-endian hardware?

lu
John Stebbins March 1, 2017, 9:40 p.m. | #2
On 03/01/2017 10:38 AM, Luca Barbato wrote:
> On 01/03/2017 16:30, John Stebbins wrote:
>> This fixes decode of rawvideo in matroska.
>> ---
>>  libavformat/matroskadec.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 4e121b6..75cfa85 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -1612,6 +1612,7 @@ static int matroska_parse_tracks(AVFormatContext *s)
>>                  track->video.display_width = track->video.pixel_width;
>>              if (!track->video.display_height)
>>                  track->video.display_height = track->video.pixel_height;
>> +            track->video.fourcc = av_bswap32(track->video.fourcc);
>>          } else if (track->type == MATROSKA_TRACK_TYPE_AUDIO) {
>>              if (!track->audio.out_samplerate)
>>                  track->audio.out_samplerate = track->audio.samplerate;
>>
> Would be nice to have some more information in the commit message.
>
> Are you sure you want to swap it even when you are on big-endian hardware?
>
>

Hmm, I better test.  I'm not sure about that.  I'll do an oracle run when I have some time.  Next few days are hit and
miss for me.
John Stebbins March 5, 2017, 4:18 p.m. | #3
On 03/01/2017 02:40 PM, John Stebbins wrote:
> On 03/01/2017 10:38 AM, Luca Barbato wrote:
>> On 01/03/2017 16:30, John Stebbins wrote:
>>> This fixes decode of rawvideo in matroska.
>>> ---
>>>  libavformat/matroskadec.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>> index 4e121b6..75cfa85 100644
>>> --- a/libavformat/matroskadec.c
>>> +++ b/libavformat/matroskadec.c
>>> @@ -1612,6 +1612,7 @@ static int matroska_parse_tracks(AVFormatContext *s)
>>>                  track->video.display_width = track->video.pixel_width;
>>>              if (!track->video.display_height)
>>>                  track->video.display_height = track->video.pixel_height;
>>> +            track->video.fourcc = av_bswap32(track->video.fourcc);
>>>          } else if (track->type == MATROSKA_TRACK_TYPE_AUDIO) {
>>>              if (!track->audio.out_samplerate)
>>>                  track->audio.out_samplerate = track->audio.samplerate;
>>>
>> Would be nice to have some more information in the commit message.
>>
>> Are you sure you want to swap it even when you are on big-endian hardware?
>>
>>
> Hmm, I better test.  I'm not sure about that.  I'll do an oracle run when I have some time.  Next few days are hit and
> miss for me.
>

I still want to do an oracle test of this, mostly for the experience since it will require adding a fate test.  But I
took a couple of minutes to look at the code and verified that bswap is correct.  MKTAG always puts the first character
in the LSB and ebml_read_uint always puts the first character in the MSB.

Patch

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 4e121b6..75cfa85 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1612,6 +1612,7 @@  static int matroska_parse_tracks(AVFormatContext *s)
                 track->video.display_width = track->video.pixel_width;
             if (!track->video.display_height)
                 track->video.display_height = track->video.pixel_height;
+            track->video.fourcc = av_bswap32(track->video.fourcc);
         } else if (track->type == MATROSKA_TRACK_TYPE_AUDIO) {
             if (!track->audio.out_samplerate)
                 track->audio.out_samplerate = track->audio.samplerate;