diff mbox series

[FFmpeg-devel,1/9] avformat/s337m: Use base AVClass for av_log usage

Message ID 20200103155636.7476-2-nicolas.gaullier@cji.paris
State Superseded
Headers show
Series avformat: Use s337m subdemux inside wav | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Nicolas Gaullier Jan. 3, 2020, 3:56 p.m. UTC 
s337m_get_offset_and_codec does not make use of
AVFormatContext: AVClass is enough for logging.
Will facilitate further use from outside
---
 libavformat/s337m.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Tomas Härdin Jan. 12, 2020, 8:13 p.m. UTC | #1 
fre 2020-01-03 klockan 16:56 +0100 skrev Nicolas Gaullier:
> s337m_get_offset_and_codec does not make use of
> AVFormatContext: AVClass is enough for logging.
> Will facilitate further use from outside
> ---
>  libavformat/s337m.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/s337m.c b/libavformat/s337m.c
> index 48ab66a6da..8956afb23f 100644
> --- a/libavformat/s337m.c
> +++ b/libavformat/s337m.c
> @@ -31,7 +31,7 @@
>  #define IS_24LE_MARKER(state)   ((state & 0xFFFFFFFFFFFF) == MARKER_24LE)
>  #define IS_LE_MARKER(state)     (IS_16LE_MARKER(state) || IS_20LE_MARKER(state) || IS_24LE_MARKER(state))
>  
> -static int s337m_get_offset_and_codec(AVFormatContext *s,
> +static int s337m_get_offset_and_codec(void *avc,
>                                        uint64_t state,
>                                        int data_type, int data_size,
>                                        int *offset, enum AVCodecID *codec)
> @@ -50,8 +50,8 @@ static int s337m_get_offset_and_codec(AVFormatContext *s,
>      }
>  
>      if ((data_type & 0x1F) != 0x1C) {
> -        if (s)
> -            avpriv_report_missing_feature(s, "Data type %#x in SMPTE 337M", data_type & 0x1F);
> +        if (avc)
> +            avpriv_report_missing_feature(avc, "Data type %#x in SMPTE 337M", data_type & 0x1F);
>          return AVERROR_PATCHWELCOME;
>      }
>  
> @@ -72,8 +72,8 @@ static int s337m_get_offset_and_codec(AVFormatContext *s,
>          *offset = 1601;
>          break;
>      default:
> -        if (s)
> -            avpriv_report_missing_feature(s, "Dolby E data size %d in SMPTE 337M", data_size);
> +        if (avc)
> +            avpriv_report_missing_feature(avc, "Dolby E data size %d in SMPTE 337M", data_size);
>          return AVERROR_PATCHWELCOME;
>      }
>  

Should be ok

/Tomas
Michael Niedermayer Jan. 14, 2020, 5:02 p.m. UTC | #2 
On Sun, Jan 12, 2020 at 09:13:49PM +0100, Tomas Härdin wrote:
> fre 2020-01-03 klockan 16:56 +0100 skrev Nicolas Gaullier:
> > s337m_get_offset_and_codec does not make use of
> > AVFormatContext: AVClass is enough for logging.
> > Will facilitate further use from outside
> > ---
> >  libavformat/s337m.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavformat/s337m.c b/libavformat/s337m.c
> > index 48ab66a6da..8956afb23f 100644
> > --- a/libavformat/s337m.c
> > +++ b/libavformat/s337m.c
> > @@ -31,7 +31,7 @@
> >  #define IS_24LE_MARKER(state)   ((state & 0xFFFFFFFFFFFF) == MARKER_24LE)
> >  #define IS_LE_MARKER(state)     (IS_16LE_MARKER(state) || IS_20LE_MARKER(state) || IS_24LE_MARKER(state))
> >  
> > -static int s337m_get_offset_and_codec(AVFormatContext *s,
> > +static int s337m_get_offset_and_codec(void *avc,
> >                                        uint64_t state,
> >                                        int data_type, int data_size,
> >                                        int *offset, enum AVCodecID *codec)
> > @@ -50,8 +50,8 @@ static int s337m_get_offset_and_codec(AVFormatContext *s,
> >      }
> >  
> >      if ((data_type & 0x1F) != 0x1C) {
> > -        if (s)
> > -            avpriv_report_missing_feature(s, "Data type %#x in SMPTE 337M", data_type & 0x1F);
> > +        if (avc)
> > +            avpriv_report_missing_feature(avc, "Data type %#x in SMPTE 337M", data_type & 0x1F);
> >          return AVERROR_PATCHWELCOME;
> >      }
> >  
> > @@ -72,8 +72,8 @@ static int s337m_get_offset_and_codec(AVFormatContext *s,
> >          *offset = 1601;
> >          break;
> >      default:
> > -        if (s)
> > -            avpriv_report_missing_feature(s, "Dolby E data size %d in SMPTE 337M", data_size);
> > +        if (avc)
> > +            avpriv_report_missing_feature(avc, "Dolby E data size %d in SMPTE 337M", data_size);
> >          return AVERROR_PATCHWELCOME;
> >      }
> >  
> 
> Should be ok

should i apply this or wait for the rest of the set ?

thx

[...]
Tomas Härdin Jan. 14, 2020, 5:26 p.m. UTC | #3 
tis 2020-01-14 klockan 18:02 +0100 skrev Michael Niedermayer:
> On Sun, Jan 12, 2020 at 09:13:49PM +0100, Tomas Härdin wrote:
> > fre 2020-01-03 klockan 16:56 +0100 skrev Nicolas Gaullier:
> > > s337m_get_offset_and_codec does not make use of
> > > AVFormatContext: AVClass is enough for logging.
> > > Will facilitate further use from outside
> > > ---
> > >  libavformat/s337m.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/libavformat/s337m.c b/libavformat/s337m.c
> > > index 48ab66a6da..8956afb23f 100644
> > > --- a/libavformat/s337m.c
> > > +++ b/libavformat/s337m.c
> > > @@ -31,7 +31,7 @@
> > >  #define IS_24LE_MARKER(state)   ((state & 0xFFFFFFFFFFFF) == MARKER_24LE)
> > >  #define IS_LE_MARKER(state)     (IS_16LE_MARKER(state) || IS_20LE_MARKER(state) || IS_24LE_MARKER(state))
> > >  
> > > -static int s337m_get_offset_and_codec(AVFormatContext *s,
> > > +static int s337m_get_offset_and_codec(void *avc,
> > >                                        uint64_t state,
> > >                                        int data_type, int data_size,
> > >                                        int *offset, enum AVCodecID *codec)
> > > @@ -50,8 +50,8 @@ static int s337m_get_offset_and_codec(AVFormatContext *s,
> > >      }
> > >  
> > >      if ((data_type & 0x1F) != 0x1C) {
> > > -        if (s)
> > > -            avpriv_report_missing_feature(s, "Data type %#x in SMPTE 337M", data_type & 0x1F);
> > > +        if (avc)
> > > +            avpriv_report_missing_feature(avc, "Data type %#x in SMPTE 337M", data_type & 0x1F);
> > >          return AVERROR_PATCHWELCOME;
> > >      }
> > >  
> > > @@ -72,8 +72,8 @@ static int s337m_get_offset_and_codec(AVFormatContext *s,
> > >          *offset = 1601;
> > >          break;
> > >      default:
> > > -        if (s)
> > > -            avpriv_report_missing_feature(s, "Dolby E data size %d in SMPTE 337M", data_size);
> > > +        if (avc)
> > > +            avpriv_report_missing_feature(avc, "Dolby E data size %d in SMPTE 337M", data_size);
> > >          return AVERROR_PATCHWELCOME;
> > >      }
> > >  
> > 
> > Should be ok
> 
> should i apply this or wait for the rest of the set ?

Carl had some thoughts that should be addressed I think. It also
sounded like Nicolas was working on a new patchset

/Tomas
Nicolas Gaullier Jan. 14, 2020, 10:43 p.m. UTC | #4 
>> fre 2020-01-03 klockan 16:56 +0100 skrev Nicolas Gaullier:
>> > s337m_get_offset_and_codec does not make use of
>> > AVFormatContext: AVClass is enough for logging.
>> > Will facilitate further use from outside
>> > ---
>> >  libavformat/s337m.c | 10 +++++-----
>> >  1 file changed, 5 insertions(+), 5 deletions(-)
>> > 
>> > diff --git a/libavformat/s337m.c b/libavformat/s337m.c index 
>> > 48ab66a6da..8956afb23f 100644
>> > --- a/libavformat/s337m.c
>> > +++ b/libavformat/s337m.c
>> > @@ -31,7 +31,7 @@
>> >  #define IS_24LE_MARKER(state)   ((state & 0xFFFFFFFFFFFF) == MARKER_24LE)
>> >  #define IS_LE_MARKER(state)     (IS_16LE_MARKER(state) || IS_20LE_MARKER(state) || IS_24LE_MARKER(state))
>> >  
>> > -static int s337m_get_offset_and_codec(AVFormatContext *s,
>> > +static int s337m_get_offset_and_codec(void *avc,
>> >                                        uint64_t state,
>> >                                        int data_type, int data_size,
>> >                                        int *offset, enum AVCodecID 
>> > *codec) @@ -50,8 +50,8 @@ static int s337m_get_offset_and_codec(AVFormatContext *s,
>> >      }
>> >  
>> >      if ((data_type & 0x1F) != 0x1C) {
>> > -        if (s)
>> > -            avpriv_report_missing_feature(s, "Data type %#x in SMPTE 337M", data_type & 0x1F);
>> > +        if (avc)
>> > +            avpriv_report_missing_feature(avc, "Data type %#x in 
>> > + SMPTE 337M", data_type & 0x1F);
>> >          return AVERROR_PATCHWELCOME;
>> >      }
>> >  
>> > @@ -72,8 +72,8 @@ static int s337m_get_offset_and_codec(AVFormatContext *s,
>> >          *offset = 1601;
>> >          break;
>> >      default:
>> > -        if (s)
>> > -            avpriv_report_missing_feature(s, "Dolby E data size %d in SMPTE 337M", data_size);
>> > +        if (avc)
>> > +            avpriv_report_missing_feature(avc, "Dolby E data size 
>> > + %d in SMPTE 337M", data_size);
>> >          return AVERROR_PATCHWELCOME;
>> >      }
>> >  
>> 
>> Should be ok
>
>should i apply this or wait for the rest of the set ?

In my opinion, it can be applied. The idea is to have something consistent with later introduced ff_s337m_get_packet() parameters and those have not been questioned in the current review.

Nicolas
diff mbox series

Patch

diff --git a/libavformat/s337m.c b/libavformat/s337m.c
index 48ab66a6da..8956afb23f 100644
--- a/libavformat/s337m.c
+++ b/libavformat/s337m.c
@@ -31,7 +31,7 @@ 
 #define IS_24LE_MARKER(state)   ((state & 0xFFFFFFFFFFFF) == MARKER_24LE)
 #define IS_LE_MARKER(state)     (IS_16LE_MARKER(state) || IS_20LE_MARKER(state) || IS_24LE_MARKER(state))
 
-static int s337m_get_offset_and_codec(AVFormatContext *s,
+static int s337m_get_offset_and_codec(void *avc,
                                       uint64_t state,
                                       int data_type, int data_size,
                                       int *offset, enum AVCodecID *codec)
@@ -50,8 +50,8 @@  static int s337m_get_offset_and_codec(AVFormatContext *s,
     }
 
     if ((data_type & 0x1F) != 0x1C) {
-        if (s)
-            avpriv_report_missing_feature(s, "Data type %#x in SMPTE 337M", data_type & 0x1F);
+        if (avc)
+            avpriv_report_missing_feature(avc, "Data type %#x in SMPTE 337M", data_type & 0x1F);
         return AVERROR_PATCHWELCOME;
     }
 
@@ -72,8 +72,8 @@  static int s337m_get_offset_and_codec(AVFormatContext *s,
         *offset = 1601;
         break;
     default:
-        if (s)
-            avpriv_report_missing_feature(s, "Dolby E data size %d in SMPTE 337M", data_size);
+        if (avc)
+            avpriv_report_missing_feature(avc, "Dolby E data size %d in SMPTE 337M", data_size);
         return AVERROR_PATCHWELCOME;
     }