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

David Conrad lessen42
Fri May 22 11:52:53 CEST 2009


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.



More information about the ffmpeg-devel mailing list