[FFmpeg-devel] [PATCH] Chromium MKV patches

[David Conrad]

On May 9, 2010, at 12:35 PM, Aurelien Jacobs wrote:

> On Wed, May 05, 2010 at 05:18:20AM -0400, David Conrad wrote:
>> Hi,
>> 
>> Here are the useful patches from chromium for the mkv demuxer. 1 doesn't fix any actual issues that I'm aware of, but might make it easier to spot any if they exist.
>> 
>> Another change made was ensuring sample_rate was nonzero, but AFAIK a zero sample_rate meant unknown and applications should handle that instead of the demuxer inventing a rate?
>> 
> 
>> From ccd07e7b93df77882cd5f69c00c32af728a6545e Mon Sep 17 00:00:00 2001
>> From: David Conrad <lessen42 at gmail.com>
>> Date: Sat, 1 May 2010 14:05:14 -0400
>> Subject: [PATCH 1/4] matroskadec: Use av_freep in ebml_read_ascii
>> 
>> Based on a Chromium patch
>> ---
>> libavformat/matroskadec.c |    4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 4e6f375..46d039a 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -615,13 +615,13 @@ static int ebml_read_float(ByteIOContext *pb, int size, double *num)
>>  */
>> static int ebml_read_ascii(ByteIOContext *pb, int size, char **str)
>> {
>> -    av_free(*str);
>> +    av_freep(str);
>>     /* EBML strings are usually not 0-terminated, so we allocate one
>>      * byte more, read the string and NULL-terminate it ourselves. */
>>     if (!(*str = av_malloc(size + 1)))
>>         return AVERROR(ENOMEM);
> 
> This hunk is rejected.
> There is no point in zeroing *str when it's overwritten just on the next
> line.

Removed

>>     if (get_buffer(pb, (uint8_t *) *str, size) != size) {
>> -        av_free(*str);
>> +        av_freep(str);
>>         return AVERROR(EIO);
>>     }
>>     (*str)[size] = '\0';
> 
> Might make debugging easier if something wrong happen, so OK.

Applied

>> From fd0192fc730621b16f851d1e47bc854a63b1d4a9 Mon Sep 17 00:00:00 2001
>> From: David Conrad <lessen42 at gmail.com>
>> Date: Sat, 1 May 2010 14:13:01 -0400
>> Subject: [PATCH 2/4] matroskadec: Ensure time_scale is nonzero, fixes divide-by-zero if the file
>> has 0 written
>> 
>> Based on a Chromium patch
>> ---
>> libavformat/matroskadec.c |    2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>> 
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 46d039a..2b4d513 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -1154,6 +1154,8 @@ static int matroska_read_header(AVFormatContext *s, AVFormatParameters *ap)
>>         return -1;
>>     matroska_execute_seekhead(matroska);
>> 
>> +    if (!matroska->time_scale)
>> +        matroska->time_scale = 1000000;
>>     if (matroska->duration)
>>         matroska->ctx->duration = matroska->duration * matroska->time_scale
>>                                   * 1000 / AV_TIME_BASE;
> 
> A file containing time_scale=0 is broken.
> But if they wrote this patch, I guess such a file existe in the wild, so
> patch OK.
> Having a sample would of course be useful...

Easy enough to create one by writing 0 here in matroskaenc.
Applied.

>> From 3c8b0a5a4f3843f0f6f9d31ac8e8a486ced27d62 Mon Sep 17 00:00:00 2001
>> From: David Conrad <lessen42 at gmail.com>
>> Date: Sat, 1 May 2010 14:17:57 -0400
>> Subject: [PATCH 3/4] matroskadec: Fix buffer overread in matroska_ebmlnum_uint
>> 
>> Based on a Chromium patch
>> ---
>> libavformat/matroskadec.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 2b4d513..8e70bbc 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -679,7 +679,7 @@ static int matroska_ebmlnum_uint(MatroskaDemuxContext *matroska,
>> {
>>     ByteIOContext pb;
>>     init_put_byte(&pb, data, size, 0, NULL, NULL, NULL, NULL);
>> -    return ebml_read_num(matroska, &pb, 8, num);
>> +    return ebml_read_num(matroska, &pb, FFMIN(size, 8), num);
>> }
>> 
>> /*
> 
> Should be OK.

Applied

>> From 35c4685b5b08f961522ead0a7b4b534dce552ff6 Mon Sep 17 00:00:00 2001
>> From: David Conrad <lessen42 at gmail.com>
>> Date: Sat, 1 May 2010 14:42:12 -0400
>> Subject: [PATCH 4/4] matroskadec: Free ebml binary buffer on error
>> 
>> Based on a Chromium patch
>> ---
>> libavformat/matroskadec.c |    4 +++-
>> 1 files changed, 3 insertions(+), 1 deletions(-)
>> 
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 8e70bbc..e449838 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -641,8 +641,10 @@ static int ebml_read_binary(ByteIOContext *pb, int length, EbmlBin *bin)
>> 
>>     bin->size = length;
>>     bin->pos  = url_ftell(pb);
>> -    if (get_buffer(pb, bin->data, length) != length)
>> +    if (get_buffer(pb, bin->data, length) != length) {
>> +        av_freep(&bin->data);
>>         return AVERROR(EIO);
>> +    }
>> 
>>     return 0;
>> }
> 
> OK.

Applied