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

[Baptiste Coudurier]

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.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org