diff mbox

[FFmpeg-devel,3/3] avformat/utils: Slightly un-clutter code in determinable_frame_size() by using a switch

Message ID 20170604002546.5707-3-michael@niedermayer.cc
State Accepted
Commit 850cbd496f8d583467cf6194dec3f6264efc0521
Headers show

Commit Message

Michael Niedermayer June 4, 2017, 12:25 a.m. UTC 
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/utils.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

wm4 June 6, 2017, 1:57 p.m. UTC | #1 
On Sun,  4 Jun 2017 02:25:46 +0200
Michael Niedermayer <michael@niedermayer.cc> wrote:

> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/utils.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index c5f1eac185..bbc7a7b547 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -894,12 +894,13 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>  
>  static int determinable_frame_size(AVCodecContext *avctx)
>  {
> -    if (/*avctx->codec_id == AV_CODEC_ID_AAC ||*/
> -        avctx->codec_id == AV_CODEC_ID_MP1 ||
> -        avctx->codec_id == AV_CODEC_ID_MP2 ||
> -        avctx->codec_id == AV_CODEC_ID_MP3/* ||
> -        avctx->codec_id == AV_CODEC_ID_CELT*/)
> +    switch(avctx->codec_id) {
> +    case AV_CODEC_ID_MP1:
> +    case AV_CODEC_ID_MP2:
> +    case AV_CODEC_ID_MP3:
>          return 1;
> +    }
> +
>      return 0;
>  }
>  

Seems like an improvement.

Is there any reason for this function to exist, though? (I bet nobody
knows.)
Michael Niedermayer June 6, 2017, 3:27 p.m. UTC | #2 
On Tue, Jun 06, 2017 at 03:57:22PM +0200, wm4 wrote:
> On Sun,  4 Jun 2017 02:25:46 +0200
> Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/utils.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index c5f1eac185..bbc7a7b547 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -894,12 +894,13 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
> >  
> >  static int determinable_frame_size(AVCodecContext *avctx)
> >  {
> > -    if (/*avctx->codec_id == AV_CODEC_ID_AAC ||*/
> > -        avctx->codec_id == AV_CODEC_ID_MP1 ||
> > -        avctx->codec_id == AV_CODEC_ID_MP2 ||
> > -        avctx->codec_id == AV_CODEC_ID_MP3/* ||
> > -        avctx->codec_id == AV_CODEC_ID_CELT*/)
> > +    switch(avctx->codec_id) {
> > +    case AV_CODEC_ID_MP1:
> > +    case AV_CODEC_ID_MP2:
> > +    case AV_CODEC_ID_MP3:
> >          return 1;
> > +    }
> > +
> >      return 0;
> >  }
> >  
> 
> Seems like an improvement.

applied


> 
> Is there any reason for this function to exist, though? (I bet nobody
> knows.)

If i remember correctly
It lists the audio codecs for which the parser will set the frame
size upon encountering a valid undamaged (key) frame.
I think some streams require waiting for this point to have correct
information.

[...]
diff mbox

Patch

diff --git a/libavformat/utils.c b/libavformat/utils.c
index c5f1eac185..bbc7a7b547 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -894,12 +894,13 @@  int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
 
 static int determinable_frame_size(AVCodecContext *avctx)
 {
-    if (/*avctx->codec_id == AV_CODEC_ID_AAC ||*/
-        avctx->codec_id == AV_CODEC_ID_MP1 ||
-        avctx->codec_id == AV_CODEC_ID_MP2 ||
-        avctx->codec_id == AV_CODEC_ID_MP3/* ||
-        avctx->codec_id == AV_CODEC_ID_CELT*/)
+    switch(avctx->codec_id) {
+    case AV_CODEC_ID_MP1:
+    case AV_CODEC_ID_MP2:
+    case AV_CODEC_ID_MP3:
         return 1;
+    }
+
     return 0;
 }