[FFmpeg-devel] [PATCH] avformat: export probe score

[Paul B Mahol]

On 8/27/13, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Fri, Aug 09, 2013 at 10:39:49AM +0200, Stefano Sabatini wrote:
>> On date Thursday 2013-08-08 22:45:28 +0200, Michael Niedermayer encoded:
>> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>> > ---
>> >  libavformat/avformat.h      |   17 +++++++++++++++--
>> >  libavformat/options_table.h |    1 +
>> >  libavformat/utils.c         |   18 ++++++++++++++----
>> >  3 files changed, 30 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> > index d5f8a29..6afdcf5 100644
>> > --- a/libavformat/avformat.h
>> > +++ b/libavformat/avformat.h
>> > @@ -1237,6 +1237,13 @@ typedef struct AVFormatContext {
>> >       */
>> >      int flush_packets;
>> >
>> > +    /**
>> > +     * format probing score
>> > +     * - encoding: unused
>> > +     * - decoding: set by avformat, read by user via AVOPtions (NO
>> > direct access)
>>
>> AVOptions
>>
>> And yes this is weird, since this is not anymore an option but
>> something else.
>>
>> > +     */
>> > +    int probe_score;
>> > +
>> >      /*****************************************************************
>> >       * All fields below this line are not part of the public API. They
>> >       * may not be used outside of libavformat and can be changed and
>> > @@ -1510,13 +1517,19 @@ AVInputFormat
>> > *av_probe_input_format3(AVProbeData *pd, int is_opened, int *score
>> >   * @param logctx the log context
>> >   * @param offset the offset within the bytestream to probe from
>> >   * @param max_probe_size the maximum probe buffer size (zero for
>> > default)
>> > - * @return 0 in case of success, a negative value corresponding to an
>> > + * @return the score in case of success, a negative value corresponding
>> > to an
>> >   * AVERROR code otherwise
>> >   */
>>
>> Maybe add a note about the range, so the interested user won't have to
>> read sourcecode.
>>
>> > +int av_probe_input_buffer2(AVIOContext *pb, AVInputFormat **fmt,
>> > +                           const char *filename, void *logctx,
>> > +                           unsigned int offset, unsigned int
>> > max_probe_size);
>> > +
>> > +/**
>> > + * Like av_probe_input_buffer2() but returns 0 on success
>> > + */
>> >  int av_probe_input_buffer(AVIOContext *pb, AVInputFormat **fmt,
>> >                            const char *filename, void *logctx,
>> >                            unsigned int offset, unsigned int
>> > max_probe_size);
>> > -
>> >  /**
>> >   * Open an input stream and read the header. The codecs are not
>> > opened.
>> >   * The stream must be closed with av_close_input_file().
>> > diff --git a/libavformat/options_table.h b/libavformat/options_table.h
>> > index a87868e..cf7f2da 100644
>> > --- a/libavformat/options_table.h
>> > +++ b/libavformat/options_table.h
>> > @@ -76,6 +76,7 @@ static const AVOption avformat_options[] = {
>> >  {"skip_initial_bytes", "skip initial bytes",
>> > OFFSET(skip_initial_bytes), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX-1,
>> > D},
>> >  {"correct_ts_overflow", "correct single timestamp overflows",
>> > OFFSET(correct_ts_overflow), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, D},
>> >  {"flush_packets", "enable flushing of the I/O context after each
>> > packet", OFFSET(flush_packets), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, E},
>>
>> > +{"probe_score", "score with which the format was probed",
>> > OFFSET(probe_score), AV_OPT_TYPE_INT, {.i64 = 0}, 0, AVPROBE_SCORE_MAX,
>> > D},
>>
>> Options are usually used to set an internal value, not to read values
>> set internally, thus this is a semantical inconsistency.
>
> huh?
> AVFrame.best_effort_timestam
> AVFrame.pkt_pos
> AVCodecContext emu_edge, input_preserved, frame_number, mv_bits
> i/p_tex_bits and many others
>
>
>>
>> Also it should be "set score...", which doesn't make sense in this
>
> honestly it never made sense, these all represent fields of structures
> not code setting or getting anything.
>
>
>> case since it is read-only. Also missing documentation in
>> doc/formats.texi.
>
> done, also all other changes
>

Can you show updated patch?

I'm against doing this via AVOption as its is bad and ugly.

>
>>
>> Probably adding a function to read the internal score value would be
>> saner.
>
> IMHO
> the API is complex enough as it is, no need for more wraper functions
> but iam not stoping anyone from adding such functions if someone wants
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The bravest are surely those who have the clearest vision
> of what is before them, glory and danger alike, and yet
> notwithstanding go out to meet it. -- Thucydides
>