[FFmpeg-devel] [PATCH] Merge (vp3|theora)_calculate_pixel_addresses

[David Conrad]

On Oct 4, 2008, at 6:36 PM, Michael Niedermayer wrote:

> On Tue, Sep 30, 2008 at 11:47:06PM -0400, David Conrad wrote:
>> On Sep 30, 2008, at 9:13 PM, David Conrad wrote:
>>
>>> On Sep 30, 2008, at 3:32 PM, matthieu castet wrote:
>>>
>>>> David Conrad wrote:
>>>>> Hi,
>>>>>
>>>>> As noted in the FIXME.
>>>>>
>>>>> On a side note, does anyone have any Theora files from before the
>>>>> bitstream finalization?
>>>> You've got : http://samples.mplayerhq.hu/ogg/Theora/theora.ogg
>>>
>>> Looks like I broke this about a year and a half ago in r8506, then  
>>> it was
>>> further broken in r8928.
>>> Attached fixes the second break.
>>
>> I need to pay more attention to warnings. Fixed version of patch 1
>> attached. Also attached is a fix for the first issue.
>
> [...]
>> diff --git a/libavcodec/xiph.c b/libavcodec/xiph.c
>> index 6554264..11caed4 100644
>> --- a/libavcodec/xiph.c
>> +++ b/libavcodec/xiph.c
>> @@ -18,6 +18,7 @@
>>  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
>> 02110-1301 USA
>>  */
>>
>> +#include "avcodec.h"
>> #include "xiph.h"
>>
>> int ff_split_xiph_headers(uint8_t *extradata, int extradata_size,
>> @@ -25,6 +26,10 @@ int ff_split_xiph_headers(uint8_t *extradata,  
>> int extradata_size,
>>                           int header_len[3])
>> {
>>     int i;
>> +    for (i = 0; i < 3; i++) {
>> +        header_start[i] = extradata;
>> +        header_len[i] = 0;
>> +    }
>>
>>     if (extradata_size >= 6 && AV_RB16(extradata) ==  
>> first_header_size) {
>>         int overall_len = 6;
>> @@ -33,8 +38,10 @@ int ff_split_xiph_headers(uint8_t *extradata,  
>> int extradata_size,
>>             extradata += 2;
>>             header_start[i] = extradata;
>>             extradata += header_len[i];
>> -            if (overall_len > extradata_size - header_len[i])
>> -                return -1;
>> +            if (overall_len > extradata_size - header_len[i]) {
>> +                av_log(NULL, AV_LOG_WARNING, "Only found %d Xiph  
>> headers, possibly old Theora\n", i+1);
>> +                return 0;
>> +            }
>>             overall_len += header_len[i];
>>         }
>
> I dont see how this can work correctly,
> if header_len isset to a too large value then instead of failing it  
> returns
> success, this isnt correct.

You're right; it only worked because overall_len was initialized  
assuming that 6 bytes in the extradata were used for header sizes,  
triggering this condition incorrectly.

Attached should be more correct overall.

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: theora-old-header.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081006/86022b22/attachment.txt>
-------------- next part --------------