flvdec: Ignore the index if it's from a creator known to be different

Message ID 1316614524-10206-1-git-send-email-martin@martin.st
State Committed
Commit bafff1668c6bc4d1cb3b7e4b9dac85b8b52e4765
Headers show

Commit Message

Martin Storsjö Sept. 21, 2011, 2:15 p.m.
---
 libavformat/flvdec.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

Comments

Luca Barbato Sept. 21, 2011, 4:54 p.m. | #1
On 9/21/11 7:15 AM, Martin Storsjö wrote:
> ---
>   libavformat/flvdec.c |   12 ++++++++++++
>   1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index cafbeb5..569d734 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -140,6 +140,18 @@ static int parse_keyframes_index(AVFormatContext *s, AVIOContext *ioc, AVStream
>       int64_t *filepositions = NULL;
>       int ret = AVERROR(ENOSYS);
>       int64_t initial_pos = avio_tell(ioc);
> +    AVDictionaryEntry *creator = av_dict_get(s->metadata, "metadatacreator",
> +                                             NULL, 0);
> +
> +    if (creator&&  !strcmp(creator->value, "MEGA")) {
> +        /* Files with this metadatacreator tag seem to have filepositions
> +         * pointing at the 4 trailer bytes of the previous packet,
> +         * which isn't the norm (nor what we expect here, nor what
> +         * jwplayer + lighttpd expect, nor what flvtool2 produces).
> +         * Just ignore the index in this case, instead of risking trying
> +         * to adjust it to something that might or might not work. */
> +        return 0;
> +    }
>

Not sure if might make sense adding a private option to disable this 
quirkmode. is this creator widespread enough to be useful to support it?

lu
Martin Storsjö Sept. 21, 2011, 6:27 p.m. | #2
On Wed, 21 Sep 2011, Luca Barbato wrote:

> On 9/21/11 7:15 AM, Martin Storsjö wrote:
>> ---
>>   libavformat/flvdec.c |   12 ++++++++++++
>>   1 files changed, 12 insertions(+), 0 deletions(-)
>> 
>> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
>> index cafbeb5..569d734 100644
>> --- a/libavformat/flvdec.c
>> +++ b/libavformat/flvdec.c
>> @@ -140,6 +140,18 @@ static int parse_keyframes_index(AVFormatContext *s, 
>> AVIOContext *ioc, AVStream
>>       int64_t *filepositions = NULL;
>>       int ret = AVERROR(ENOSYS);
>>       int64_t initial_pos = avio_tell(ioc);
>> +    AVDictionaryEntry *creator = av_dict_get(s->metadata, 
>> "metadatacreator",
>> +                                             NULL, 0);
>> +
>> +    if (creator&&  !strcmp(creator->value, "MEGA")) {
>> +        /* Files with this metadatacreator tag seem to have filepositions
>> +         * pointing at the 4 trailer bytes of the previous packet,
>> +         * which isn't the norm (nor what we expect here, nor what
>> +         * jwplayer + lighttpd expect, nor what flvtool2 produces).
>> +         * Just ignore the index in this case, instead of risking trying
>> +         * to adjust it to something that might or might not work. */
>> +        return 0;
>> +    }
>> 
>
> Not sure if might make sense adding a private option to disable this 
> quirkmode. is this creator widespread enough to be useful to support it?

I'm not sure how widespread it is - j-b mentioned it on irc that seeking 
now was broken in a sample file that he used to test, that used to work 
(before we started parsing the broken index in the file).

// Martin
Luca Barbato Sept. 21, 2011, 7:04 p.m. | #3
On 9/21/11 9:54 AM, Luca Barbato wrote:
> Not sure if might make sense adding a private option to disable this
> quirkmode. is this creator widespread enough to be useful to support it?

Let's land this patch for now we might add parsing support as a CodeIn 
task or as small task, I guess.

lu
Martin Storsjö Sept. 23, 2011, 6:13 p.m. | #4
On Wed, 21 Sep 2011, Luca Barbato wrote:

> On 9/21/11 9:54 AM, Luca Barbato wrote:
>> Not sure if might make sense adding a private option to disable this
>> quirkmode. is this creator widespread enough to be useful to support it?
>
> Let's land this patch for now we might add parsing support as a CodeIn task 
> or as small task, I guess.

Pushed for now, feel free to suggest patches for how to disable it 
conditionally.

// Martin

Patch

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index cafbeb5..569d734 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -140,6 +140,18 @@  static int parse_keyframes_index(AVFormatContext *s, AVIOContext *ioc, AVStream
     int64_t *filepositions = NULL;
     int ret = AVERROR(ENOSYS);
     int64_t initial_pos = avio_tell(ioc);
+    AVDictionaryEntry *creator = av_dict_get(s->metadata, "metadatacreator",
+                                             NULL, 0);
+
+    if (creator && !strcmp(creator->value, "MEGA")) {
+        /* Files with this metadatacreator tag seem to have filepositions
+         * pointing at the 4 trailer bytes of the previous packet,
+         * which isn't the norm (nor what we expect here, nor what
+         * jwplayer + lighttpd expect, nor what flvtool2 produces).
+         * Just ignore the index in this case, instead of risking trying
+         * to adjust it to something that might or might not work. */
+        return 0;
+    }
 
     while (avio_tell(ioc) < max_pos - 2 && amf_get_string(ioc, str_val, sizeof(str_val)) > 0) {
         int64_t* current_array;