[FFmpeg-devel] [PATCH] Fix non-mod16 libtheora encoding

[Michael Niedermayer]

On Fri, May 22, 2009 at 05:52:53AM -0400, David Conrad wrote:
> On May 21, 2009, at 10:15 PM, Michael Niedermayer wrote:
>
>> On Thu, May 21, 2009 at 05:04:00PM -0700, Baptiste Coudurier wrote:
>>> David Conrad wrote:
>>>> On May 21, 2009, at 6:14 PM, Baptiste Coudurier wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> David Conrad wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Currently using non-mod16 sizes produces garbage.
>>>>>>
>>>>>>
>>>>>> ------------------------------------------------------------------------
>>>>>>
>>>>>> commit 7af4d351600878e1c37467837d8d10c226e2b805
>>>>>> Author: David Conrad <lessen42 at gmail.com>
>>>>>> Date:   Thu May 21 17:38:44 2009 -0400
>>>>>>
>>>>>>   Fix libtheora encoding for non-mod16 sizes
>>>>>>
>>>>>> diff --git a/libavcodec/libtheoraenc.c b/libavcodec/libtheoraenc.c
>>>>>> index dbc98e3..ef8b6aa 100644
>>>>>> --- a/libavcodec/libtheoraenc.c
>>>>>> +++ b/libavcodec/libtheoraenc.c
>>>>>> @@ -87,12 +87,12 @@ static av_cold int encode_init(AVCodecContext*
>>>>>> avc_context)
>>>>>>
>>>>>>    /* Set up the theora_info struct */
>>>>>>    theora_info_init( &t_info );
>>>>>> -    t_info.width = avc_context->width;
>>>>>> -    t_info.height = avc_context->height;
>>>>>> +    t_info.width = (avc_context->width + 15) & 0xFFFFFFF0;
>>>>>> +    t_info.height = (avc_context->height + 15) & 0xFFFFFFF0;
>>>>>>    t_info.frame_width = avc_context->width;
>>>>>>    t_info.frame_height = avc_context->height;
>>>>>>    t_info.offset_x = 0;
>>>>>> -    t_info.offset_y = 0;
>>>>>> +    t_info.offset_y = avc_context->height & 0xf;
>>>>>>    /* Swap numerator and denominator as time_base in AVCodecContext
>>>>>> gives the
>>>>>>     * time period between frames, but theora_info needs the
>>>>>> framerate.  */
>>>>>>    t_info.fps_numerator = avc_context->time_base.den;
>>>>>> @@ -186,8 +186,8 @@ static int encode_frame(
>>>>>>        return -1;
>>>>>>    }
>>>>>>
>>>>>> -    t_yuv_buffer.y_width = avc_context->width;
>>>>>> -    t_yuv_buffer.y_height = avc_context->height;
>>>>>> +    t_yuv_buffer.y_width = (avc_context->width + 15) & 0xFFFFFFF0;
>>>>>> +    t_yuv_buffer.y_height = (avc_context->height + 15) & 0xFFFFFFF0;
>>>>>>    t_yuv_buffer.y_stride = frame->linesize[0];
>>>>>>    t_yuv_buffer.uv_width = t_yuv_buffer.y_width / 2;
>>>>>>    t_yuv_buffer.uv_height = t_yuv_buffer.y_height / 2;
>>>>>
>>>>> I believe a failure is more appropriate and a message saying that
>>>>> libtheora does not support non mod 16 width/height.
>>>>
>>>> Why not simply do the little bit of work libtheora won't do?
>>>
>>> Because acting on behalf of the user is bad.
>>>
>>> Inform the user of the problem, and let him handle it by either pad or 
>>> crop
>>> as he wishes, is far better IMHO.
>>
>> if libtheora doesnt support non mod 16 then yes i fully agree that we 
>> should
>> fail rather than clip something at random behind the users back
>
> As discussed on IRC, the resulting behaviour is no different than libx264 
> or any other macroblock codec I'm aware of - the coded size is padded to 
> mod16 and the extra up to 15 pixels simply aren't displayed. The only 
> difference is that libtheora requires the caller to do this manually.

then the patch is ok, though theora is silly

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090522/358955aa/attachment.pgp>