diff mbox

[FFmpeg-devel] lavf/bit: Fix the G.729 bit auto-detection

Message ID CAB0OVGpZfcubMwu=TsDBk6_B1NCfnzH_LP9Ox3=g_jRat==H3Q@mail.gmail.com
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos Sept. 29, 2017, 5:23 p.m. UTC
Hi!

The G.729 bit auto-detection has never worked, patch attached.

Please comment, Carl Eugen

Comments

Michael Niedermayer Sept. 30, 2017, 1:19 a.m. UTC | #1
On Fri, Sep 29, 2017 at 07:23:17PM +0200, Carl Eugen Hoyos wrote:
> Hi!
> 
> The G.729 bit auto-detection has never worked, patch attached.
> 
> Please comment, Carl Eugen

>  bit.c |   23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 1eaabbb2e56d350949d0c8c439a2ba49513069f2  0001-lavf-bit-Fix-the-G.729-bit-probe-function.patch
> From 2521a9bd86d807a3e18fe930b35039beb7674b3e Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> Date: Fri, 29 Sep 2017 19:10:46 +0200
> Subject: [PATCH] lavf/bit: Fix the G.729 bit auto-detection.
> 
> ---
>  libavformat/bit.c |   23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/libavformat/bit.c b/libavformat/bit.c
> index d742a5b..f90e4a7 100644
> --- a/libavformat/bit.c
> +++ b/libavformat/bit.c
> @@ -32,20 +32,23 @@
>  #if CONFIG_BIT_DEMUXER
>  static int probe(AVProbeData *p)
>  {
> -    int i, j;
> +    int i = 0, j, valid = 0;
>  
> -    if(p->buf_size < 0x40)
> -        return 0;
> -
> -    for(i=0; i+3<p->buf_size && i< 10*0x50; ){
> -        if(AV_RL16(&p->buf[0]) != SYNC_WORD)
> +    while (2 * i + 3 < p->buf_size){
> +        if (AV_RL16(&p->buf[2 * i++]) != SYNC_WORD)
>              return 0;
> -        j=AV_RL16(&p->buf[2]);
> -        if(j!=0x40 && j!=0x50)
> +        j = AV_RL16(&p->buf[2 * i++]);
> +        if (j != 0 && j != 0x10 && j != 0x40 && j != 0x50)
>              return 0;
> -        i+=j;
> +        if (j)
> +            valid++;
> +        i += j;
>      }
> -    return AVPROBE_SCORE_EXTENSION;
> +    if (valid > 10)
> +        return AVPROBE_SCORE_MAX;

this looks like a g729 stream encapsulated i another container could
return AVPROBE_SCORE_MAX. This may be sub optimal

no more comments from me

thx

[...]
Carl Eugen Hoyos Sept. 30, 2017, 6:46 p.m. UTC | #2
2017-09-30 3:19 GMT+02:00 Michael Niedermayer <michael@niedermayer.cc>:
> On Fri, Sep 29, 2017 at 07:23:17PM +0200, Carl Eugen Hoyos wrote:
>> Hi!
>>
>> The G.729 bit auto-detection has never worked, patch attached.
>>
>> Please comment, Carl Eugen
>
>>  bit.c |   23 +++++++++++++----------
>>  1 file changed, 13 insertions(+), 10 deletions(-)
>> 1eaabbb2e56d350949d0c8c439a2ba49513069f2  0001-lavf-bit-Fix-the-G.729-bit-probe-function.patch
>> From 2521a9bd86d807a3e18fe930b35039beb7674b3e Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>> Date: Fri, 29 Sep 2017 19:10:46 +0200
>> Subject: [PATCH] lavf/bit: Fix the G.729 bit auto-detection.
>>
>> ---
>>  libavformat/bit.c |   23 +++++++++++++----------
>>  1 file changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavformat/bit.c b/libavformat/bit.c
>> index d742a5b..f90e4a7 100644
>> --- a/libavformat/bit.c
>> +++ b/libavformat/bit.c
>> @@ -32,20 +32,23 @@
>>  #if CONFIG_BIT_DEMUXER
>>  static int probe(AVProbeData *p)
>>  {
>> -    int i, j;
>> +    int i = 0, j, valid = 0;
>>
>> -    if(p->buf_size < 0x40)
>> -        return 0;
>> -
>> -    for(i=0; i+3<p->buf_size && i< 10*0x50; ){
>> -        if(AV_RL16(&p->buf[0]) != SYNC_WORD)
>> +    while (2 * i + 3 < p->buf_size){
>> +        if (AV_RL16(&p->buf[2 * i++]) != SYNC_WORD)
>>              return 0;
>> -        j=AV_RL16(&p->buf[2]);
>> -        if(j!=0x40 && j!=0x50)
>> +        j = AV_RL16(&p->buf[2 * i++]);
>> +        if (j != 0 && j != 0x10 && j != 0x40 && j != 0x50)
>>              return 0;
>> -        i+=j;
>> +        if (j)
>> +            valid++;
>> +        i += j;
>>      }
>> -    return AVPROBE_SCORE_EXTENSION;
>> +    if (valid > 10)
>> +        return AVPROBE_SCORE_MAX;
>
> this looks like a g729 stream encapsulated i another container could
> return AVPROBE_SCORE_MAX. This may be sub optimal

SYNC_WORD only exists in BIT streams, not in raw g.729 streams
as muxed in other formats.

Patch pushed.

Thank you, Carl Eugen
Michael Niedermayer Sept. 30, 2017, 9:50 p.m. UTC | #3
On Sat, Sep 30, 2017 at 08:46:28PM +0200, Carl Eugen Hoyos wrote:
> 2017-09-30 3:19 GMT+02:00 Michael Niedermayer <michael@niedermayer.cc>:
> > On Fri, Sep 29, 2017 at 07:23:17PM +0200, Carl Eugen Hoyos wrote:
> >> Hi!
> >>
> >> The G.729 bit auto-detection has never worked, patch attached.
> >>
> >> Please comment, Carl Eugen
> >
> >>  bit.c |   23 +++++++++++++----------
> >>  1 file changed, 13 insertions(+), 10 deletions(-)
> >> 1eaabbb2e56d350949d0c8c439a2ba49513069f2  0001-lavf-bit-Fix-the-G.729-bit-probe-function.patch
> >> From 2521a9bd86d807a3e18fe930b35039beb7674b3e Mon Sep 17 00:00:00 2001
> >> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> >> Date: Fri, 29 Sep 2017 19:10:46 +0200
> >> Subject: [PATCH] lavf/bit: Fix the G.729 bit auto-detection.
> >>
> >> ---
> >>  libavformat/bit.c |   23 +++++++++++++----------
> >>  1 file changed, 13 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/libavformat/bit.c b/libavformat/bit.c
> >> index d742a5b..f90e4a7 100644
> >> --- a/libavformat/bit.c
> >> +++ b/libavformat/bit.c
> >> @@ -32,20 +32,23 @@
> >>  #if CONFIG_BIT_DEMUXER
> >>  static int probe(AVProbeData *p)
> >>  {
> >> -    int i, j;
> >> +    int i = 0, j, valid = 0;
> >>
> >> -    if(p->buf_size < 0x40)
> >> -        return 0;
> >> -
> >> -    for(i=0; i+3<p->buf_size && i< 10*0x50; ){
> >> -        if(AV_RL16(&p->buf[0]) != SYNC_WORD)
> >> +    while (2 * i + 3 < p->buf_size){
> >> +        if (AV_RL16(&p->buf[2 * i++]) != SYNC_WORD)
> >>              return 0;
> >> -        j=AV_RL16(&p->buf[2]);
> >> -        if(j!=0x40 && j!=0x50)
> >> +        j = AV_RL16(&p->buf[2 * i++]);
> >> +        if (j != 0 && j != 0x10 && j != 0x40 && j != 0x50)
> >>              return 0;
> >> -        i+=j;
> >> +        if (j)
> >> +            valid++;
> >> +        i += j;
> >>      }
> >> -    return AVPROBE_SCORE_EXTENSION;
> >> +    if (valid > 10)
> >> +        return AVPROBE_SCORE_MAX;
> >
> > this looks like a g729 stream encapsulated i another container could
> > return AVPROBE_SCORE_MAX. This may be sub optimal
> 
> SYNC_WORD only exists in BIT streams, not in raw g.729 streams
> as muxed in other formats.

i dont think assuming the non existance of one kind of container to
ever be put in another is a safe assumtation.

also theres tar and other generic containers.

still this checks the files first few bytes so its probably ok as
most other containrs wouldnt match here

[...]
diff mbox

Patch

From 2521a9bd86d807a3e18fe930b35039beb7674b3e Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Fri, 29 Sep 2017 19:10:46 +0200
Subject: [PATCH] lavf/bit: Fix the G.729 bit auto-detection.

---
 libavformat/bit.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/libavformat/bit.c b/libavformat/bit.c
index d742a5b..f90e4a7 100644
--- a/libavformat/bit.c
+++ b/libavformat/bit.c
@@ -32,20 +32,23 @@ 
 #if CONFIG_BIT_DEMUXER
 static int probe(AVProbeData *p)
 {
-    int i, j;
+    int i = 0, j, valid = 0;
 
-    if(p->buf_size < 0x40)
-        return 0;
-
-    for(i=0; i+3<p->buf_size && i< 10*0x50; ){
-        if(AV_RL16(&p->buf[0]) != SYNC_WORD)
+    while (2 * i + 3 < p->buf_size){
+        if (AV_RL16(&p->buf[2 * i++]) != SYNC_WORD)
             return 0;
-        j=AV_RL16(&p->buf[2]);
-        if(j!=0x40 && j!=0x50)
+        j = AV_RL16(&p->buf[2 * i++]);
+        if (j != 0 && j != 0x10 && j != 0x40 && j != 0x50)
             return 0;
-        i+=j;
+        if (j)
+            valid++;
+        i += j;
     }
-    return AVPROBE_SCORE_EXTENSION;
+    if (valid > 10)
+        return AVPROBE_SCORE_MAX;
+    if (valid > 2)
+        return AVPROBE_SCORE_EXTENSION - 1;
+    return 0;
 }
 
 static int read_header(AVFormatContext *s)
-- 
1.7.10.4