[FFmpeg-devel] [PATCH] lavf/utils: extend has_codec_parameters() to make it show what info is missing

[Michael Niedermayer]

On Tue, Jul 17, 2012 at 10:30:56PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2012-07-17 17:00:44 +0200, Michael Niedermayer encoded:
> > On Tue, Jul 17, 2012 at 09:04:47AM +0200, Stefano Sabatini wrote:
> > > On date Monday 2012-07-16 05:04:55 +0200, Michael Niedermayer encoded:
> > > > On Mon, Jul 16, 2012 at 12:30:58AM +0200, Stefano Sabatini wrote:
> > > > > Improve feedback.
> > > > > ---
> > > > >  libavformat/utils.c |   47 ++++++++++++++++++++++++++++++-----------------
> > > > >  1 files changed, 30 insertions(+), 17 deletions(-)
> > > > > 
> > > > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > > > index 39527a6..74b6746 100644
> > > > > --- a/libavformat/utils.c
> > > > > +++ b/libavformat/utils.c
> > > > > @@ -2265,30 +2265,40 @@ static void estimate_timings(AVFormatContext *ic, int64_t old_offset)
> > > > >      }
> > > > >  }
> > > > >  
> > > > > -static int has_codec_parameters(AVStream *st)
> > > > > +static int has_codec_parameters(AVStream *st, char *error, size_t error_size)
> > > > >  {
> > > > >      AVCodecContext *avctx = st->codec;
> > > > > -    int val;
> > > > > +
> > > > > +#define FAIL(errmsg) do {                               \
> > > > > +        if (error)                                      \
> > > > > +            av_strlcpy(error, errmsg, error_size);      \
> > > > > +        return 0;                                       \
> > > > > +    } while (0)
> > > > > +
> > > > 
> > > > passing a flag into has_codec_parameters() and printing the message
> > > > in there when its != 0 seems cleaner
> > > > not sure if theres a even cleaner way
> > > 
> > > The problem is that when the codec parameters are checked the codec is
> > > not yet set, so you'll see a message of the kind:
> > > 
> > > [flv @ 0x3303160] max_analyze_duration 5000000 reached at 5018000
> > > [NULL @ 0x33095c0] Unspecified frame size
> > > [flv @ 0x3303160] Could not find codec parameters (Video: h264, 204 kb/s)
> > > 
> > > which is not very useful.
> > 
> > AVFormatContext could be passed into has_codec_parameters()
> 
> Nitfest.
> 
> Currently:
> static int has_codec_parameters(AVStream *st)
> =>
> [flv @ 0x3303160] max_analyze_duration 5000000 reached at 5018000
> [flv @ 0x3303160] Could not find codec parameters (Video: h264, 204 kb/s)
> 
> Alternative 1:
> static int has_codec_parameters(AVStream *st, void *logctx)
> =>
> [flv @ 0x3303160] max_analyze_duration 5000000 reached at 5018000
> [flv @ 0x3303160] Unspecified frame size for stream (Video: h264, 204 kb/s)
> [flv @ 0x3303160] Could not find codec parameters for stream 0 (Video: h264, 204 kb/s)
> 
> Alternative 2:
> static int has_codec_parameters(AVStream *st, char *msg, size_t msg_size)
> =>
> [flv @ 0x3303160] max_analyze_duration 5000000 reached at 5018000
> [flv @ 0x3303160] Could not find codec parameters for stream 0 (Video: h264, 204 kb/s): unspecified frame size
> 
> With alternative 1 there is no way to show the stream index, since
> has_codec_parameters() takes in input the stream, and the index may
> not be available when the function is called (may be an issue if you
> have several streams of the same type in input).
> 
> Alternative 2 provides the most useful and complete feedback IMHO, but
> I'm fine with 1 if you dislike it very much.

what about just returning a pointer to the error message through a
const char **err_message argument ?
it would avoid the strlcpy()

either way feel free to commit what you like best.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120718/82ff14f7/attachment.asc>