[FFmpeg-devel] [PATCH] Update lix264.c for x264 API change (X264_BUILD 79)

Måns Rullgård mans
Tue Nov 24 14:07:42 CET 2009


Michael Niedermayer <michaelni at gmx.at> writes:

> On Mon, Nov 23, 2009 at 08:17:40PM -0800, Jason Garrett-Glaser wrote:
>> On Mon, Nov 23, 2009 at 3:51 AM, Jason Garrett-Glaser
>> <darkshikari at gmail.com> wrote:
>> > 2009/11/23 M?ns Rullg?rd <mans at mansr.com>:
>> >> Jason Garrett-Glaser <darkshikari at gmail.com> writes:
>> >>
>> >>> 2009/11/23 M?ns Rullg?rd <mans at mansr.com>:
>> >>>> Jason Garrett-Glaser <darkshikari at gmail.com> writes:
>> >>>>
>> >>>>> On Sat, Nov 21, 2009 at 3:14 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >>>>>> On Sat, Nov 21, 2009 at 02:46:45PM -0800, Jason Garrett-Glaser wrote:
>> >>>>>>> On Sat, Nov 21, 2009 at 2:21 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >>>>>>> > On Sat, Nov 21, 2009 at 11:28:57AM -0800, Jason Garrett-Glaser wrote:
>> >>>>>>> >> OK, updated patch attached.
>> >>>>>>> > [...
>> >>>>>>> >> Index: libavcodec/avcodec.h
>> >>>>>>> >> ===================================================================
>> >>>>>>> >> --- libavcodec/avcodec.h ? ? ?(revision 20562)
>> >>>>>>> >> +++ libavcodec/avcodec.h ? ? ?(working copy)
>> >>>>>>> >> @@ -2553,6 +2553,13 @@
>> >>>>>>> >> ? ? ? * - decoding: Set by libavcodec, user can override.
>> >>>>>>> >> ? ? ? */
>> >>>>>>> >> ? ? ?int (*execute2)(struct AVCodecContext *c, int (*func)(struct AVCodecContext *c2, void *arg, int jobnr, int threadnr), void *arg2, int *ret, int count);
>> >>>>>>> >> +
>> >>>>>>> >> + ? ?/**
>> >>>>>>> >> + ? ? * explicit weighted prediction analysis method
>> >>>>>>> >> + ? ? * - encoding: Set by user.
>> >>>>>>> >> + ? ? * - decoding: unused
>> >>>>>>> >> + ? ? */
>> >>>>>>> >> + ? ?int weighted_p_pred;
>> >>>>>>> >> ?} AVCodecContext;
>> >>>>>>> >
>> >>>>>>> > This documentation is insufficient to use this field.
>> >>>>>>> > It tells the reader what the field does but not which value does
>> >>>>>>> > what
>> >>>>>>>
>> >>>>>>> How should we keep the documentation in ffmpeg synced with the values
>> >>>>>>> in x264 then? ?We may add new values in the future.
>> >>>>>>
>> >>>>>> Theres no problem with adding new values in avcodec.h, removing or changing
>> >>>>>> them is a problem, but i would suspect that is not planned.
>> >>>>>>
>> >>>>>> The idea simply is that an application that uses libavcodec (and x264 throgh
>> >>>>>> libavcodec) keeps working with new libavcodec or x264 versions unless the
>> >>>>>> major version is bumped.
>> >>>>>> Adding values (which the old application doesnt know about) is no problem
>> >>>>>> redefining their meaning would be one though possibly not a huge one
>> >>>>>
>> >>>>> Updated.
>> >>>>>
>> >>>>> +{"wpredp", "weighted prediction analysis method", OFFSET(weighted_p_pred), FF_OPT_TYPE_INT, 0, INT_MIN, INT_MAX, V|E},
>> >>>>
>> >>>> What is "wpredp" supposed to be short for?
>> >>>>
>> >>>>> Index: libavcodec/avcodec.h
>> >>>>> ===================================================================
>> >>>>> --- libavcodec/avcodec.h ? ? ?(revision 20562)
>> >>>>> +++ libavcodec/avcodec.h ? ? ?(working copy)
>> >>>>> @@ -2553,6 +2553,16 @@
>> >>>>> ? ? ? * - decoding: Set by libavcodec, user can override.
>> >>>>> ? ? ? */
>> >>>>> ? ? ?int (*execute2)(struct AVCodecContext *c, int (*func)(struct AVCodecContext *c2, void *arg, int jobnr, int threadnr), void *arg2, int *ret, int count);
>> >>>>> +
>> >>>>> + ? ?/**
>> >>>>> + ? ? * explicit weighted prediction analysis method
>> >>>>> + ? ? * 0: off
>> >>>>> + ? ? * 1: fast blind weighting (one reference duplicate with -1 offset)
>> >>>>> + ? ? * 2: smart weighting (full fade detection analysis)
>> >>>>> + ? ? * - encoding: Set by user.
>> >>>>> + ? ? * - decoding: unused
>> >>>>> + ? ? */
>> >>>>> + ? ?int weighted_p_pred;
>> >>>>> ?} AVCodecContext;
>> >>>>
>> >>>> The weighted_p_pred name looks a bit weird to me. ?What does the _p_ signify?
>> >>>
>> >>> P-frames, in the same way that weightb means weighted B-frames. ?Most
>> >>> importantly it fits with the x264 name (weightp).
>> >>
>> >> So this is only for P-frames? ?That's not at all obvious.
>> >
>> > Well, in the future, x264 could theoretically get explicit weighted
>> > prediction support for B-frames as well, though I haven't seen that
>> > used very commonly, and I would still personally call it weightp
>> > (since its primary purpose is to weight P-frames, which don't get hit
>> > by implicit weighted pred, aka weightb).
>> >
>> > I've renamed it to "explicit P-frame weighted prediction analysis
>> > method" accordingly.
>> >
>> > Dark Shikari
>> >
>> Bump?  I'd like to get this in soon.
>
> As far as iam concerned, iam fine with your patch, mans might have more
> comments, i dont know ...

No, nothing more from me.

-- 
M?ns Rullg?rd
mans at mansr.com



More information about the ffmpeg-devel mailing list