[FFmpeg-devel] [PATCH] Add support for pcm_*_BE in mov

[Tim Nicholson]

On 02/10/12 20:33, Jean First wrote:
> On Tue Oct 02 2012 12:21:29 GMT+0200 (CEST), Tim Nicholson wrote:
>> On 01/10/12 20:15, Michael Niedermayer wrote:
>>> On Mon, Oct 01, 2012 at 05:25:16PM +0100, Tim Nicholson wrote:
>>>> Currently bit depths >16 are only handled correctly if they are LE.
>>>> This patch adds support for BE variants and fixes ticket #1767
>>>>
>>>> Note I have not renamed the function in this patch which suggests a
>>>> test
>>>> for le variants only. Updating this to reflect the general nature of
>>>> the
>>>> function can be done in a separate cosmetic patch if required.
>>> this patch breaks fate
>>> [...]
>> Updated patch attached.
>> This includes new checksums to reflect the change in the mov files due
>> to the different header type.
> 
>>  From 4c16fe54575a52334d4aec8d066fa51eaebd0f7b Mon Sep 17 00:00:00 2001
>> From: Tim Nicholson<Tim.Nicholson at bbc.co.uk>
>> Date: Tue, 2 Oct 2012 11:18:04 +0100
>> Subject: [PATCH] movenc.c: Add support for >16bit BE flavours
>>
>> Currently only LE handled correctly
>> Updated fate checksums to reflect new headers
>> ---
>>   libavformat/movenc.c            |   26 +++++++++++++++++++++++---
>>   tests/ref/fate/acodec-pcm-s24be |    4 ++--
>>   tests/ref/fate/acodec-pcm-s32be |    4 ++--
>>   3 files changed, 27 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index 6de5306..ac8a349 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -288,6 +288,14 @@ static int mov_write_enda_tag(AVIOContext *pb)
>>       return 10;
>>   }
>>   +static int mov_write_enda_tag_be(AVIOContext *pb)
>> +{
>> +  avio_wb32(pb, 10);
>> +  ffio_wfourcc(pb, "enda");
>> +  avio_wb16(pb, 0); /* big endian */
>> +  return 10;
>> +}
> 
> [...]
> 
> can't mov_write_enda_tag_be be merged with mov_write_enda_tag ?
> 

It could, however I thought it would be clearer in the later code to
have a separate call, and it seemed to match the current style. Given
that mov_write_enda_tag was called only once anyway it could easily have
been written inline originally, but was presumably made separate for the
same reason.

I don't have strong views either way and am happy to change it if that
is the consensus, as I say I was trying to match the existing style.

On a separate note, I have another patch in the pipeline to fix the
incorrect setting of the "Samples per packet" field for uncompressed pcm
(currently always set to "track->enc->frame_size" but required to be "1"
in the above case. Since that patch depends upon this one I am
withholding it until any issues with this one are sorted to avoid knock
on effects.


> Jean
> [..]

-- 
Tim