Something got mangled in the email.

The concept of the patch below seems good, check for every block
candidate once you find the magic nibble.

Could you please resend the two squashed in one?

Below some more comments.

On 27/01/2017 20:06, Devesh Singh wrote:
> From 86ff714a525b9678ac61a2088e90d2b4ab5997e9 Mon Sep 17 00:00:00 2001
> From: Devesh Singh <[email protected]>
> Date: Sat, 28 Jan 2017 00:28:01 +0530
> Subject: [PATCH 2/2] BugID 555: Fix to detect gsm packet data type, made check
>  more robust
> 
> ---
>  libavformat/gsmdec.c | 61 
> +++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 58 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/gsmdec.c b/libavformat/gsmdec.c
> index 1942ac1..979318b 100644
> --- a/libavformat/gsmdec.c
> +++ b/libavformat/gsmdec.c
> @@ -28,21 +28,76 @@
>  #define GSM_BLOCK_SIZE    33
>  #define GSM_BLOCK_SAMPLES 160
>  #define GSM_SAMPLE_RATE   8000
> -#define GSM_MAGIC_NUM 0xD
> +#define GSM_MAGIC_NIBBLE 0xD
> +#define ODD 1
> +#define EVEN 0
>  
>  typedef struct GSMDemuxerContext {
>      AVClass *class;
>      int sample_rate;
>  } GSMDemuxerContext;
>  
> +#define IS_MAGIC_NUM_PRESENT(a, b) b ? (((a & 0xF0) >> 4) == 
> GSM_MAGIC_NIBBLE) : ((a & 0x0F) == GSM_MAGIC_NIBBLE)
> +
> +static int gsm_check_every_nibble(AVProbeData *p, int index_first_frame, 
> uint8_t nibble)
> +{
> +    int indx, success;

If you want to be consistent with the codebase use "i" for the index in
iterations, "ret" for the return value.

> +    
> +    success = 0x1;

0x is not strictly needed, 1 is fine.

> +
> +    for (indx = index_first_frame ; indx < p->buf_size; indx += 
> GSM_BLOCK_SIZE) {
> +
> +        if (IS_MAGIC_NUM_PRESENT(*(p->buf + indx), nibble)) {

I'd use p->buf[i]

> +            success &= 0x1;
> +
> +        } else {
> +
> +            success &= 0x0;
> +            break;
> +

this seems more complex than it should, maybe something along the lines of:

for (i = start; i < end; i += stride)
     if (!check(p->buf[i]))
         return 0;
return 1;

should work the same way.

> +    /* Check if nibble is equal to GSM_MAGIC_NIBBLE and all other nibble at 
> offset of multiples of GSM_BLOCK_SIZE 
> +       are equal to GSM_MAGIC_NIBBLE */
> +
> +    for(index_first_frame = 0; index_first_frame < GSM_BLOCK_SIZE; 
> index_first_frame++) {

Again about using i and ret for consistency.

> +
> +        if (IS_MAGIC_NUM_PRESENT(*(p->buf + index_first_frame),ODD)) 

p->buf[i]

{
> +
> +            success = gsm_check_every_nibble(p, index_first_frame, ODD);
> +
> +        } else if (IS_MAGIC_NUM_PRESENT(*(p->buf + index_first_frame), 
> EVEN)) {
> +
> +            success = gsm_check_every_nibble(p, index_first_frame, EVEN);
> +
> +        } else {
> +
> +            success = 0x0;
> +        }

this else clause is not needed.

> +       
> +        if(success & 0x1) {
> +
> +            return AVPROBE_SCORE_MAX;
> +        }
> +    }
> +
>      return 0;
>  }

That's all.

Thanks a lot

lu

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to