[FFmpeg-devel] [PATCH] bring -qpfile option of x264 to ffmpeg
Michael Niedermayer
michaelni
Sat Apr 11 19:18:42 CEST 2009
On Mon, Mar 02, 2009 at 03:03:19AM +0800, Ying Bian wrote:
> On Sat, Feb 28, 2009 at 9:13 PM, Michael Niedermayer <michaelni at gmx.at>wrote:
>
> > On Sat, Feb 28, 2009 at 06:56:15PM +0800, Ying Bian wrote:
> > > On Fri, Feb 27, 2009 at 11:25 PM, Michael Niedermayer <michaelni at gmx.at
> > >wrote:
> > >
> > > > On Fri, Feb 27, 2009 at 10:44:43PM +0800, Ying Bian wrote:
> > > > > Hi,
> > > > >
> > > > > This is a patch which brings the -qpfile option of x264 to ffmpeg.
> > > > > See the help message from "x264 --longhelp":
> > > > > --qpfile <string> Force frametypes and QPs for some or
> > all
> > > > > frames
> > > > > Format of each line: framenumber
> > frametype
> > > > QP
> > > > > QP of -1 lets x264 choose. Frametypes:
> > > > > I,i,P,B,b.
> > > > >
> > > > > I believe sometimes this is useful.
> > > > >
> > > > > - by
> > > >
> > > > > From 2a1f8d36b4895fc00c28bf014756700cf53edf04 Mon Sep 17 00:00:00
> > 2001
> > > > > From: ybian <bianying at gmail.com>
> > > > > Date: Thu, 26 Feb 2009 02:14:28 +0800
> > > > > Subject: [PATCH] bring -qpfile option of x264 to ffmpeg
> > > > >
> > > > > ---
> > > > > libavcodec/avcodec.h | 7 +++++++
> > > > > libavcodec/libx264.c | 43
> > +++++++++++++++++++++++++++++++++++++++++++
> > > > > libavcodec/options.c | 1 +
> > > > > 3 files changed, 51 insertions(+), 0 deletions(-)
> > > > >
> > > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > > index f38ad2b..38d2dd0 100644
> > > > > --- a/libavcodec/avcodec.h
> > > > > +++ b/libavcodec/avcodec.h
> > > > > @@ -2315,6 +2315,13 @@ typedef struct AVCodecContext {
> > > > > * - decoding: unused.
> > > > > */
> > > > > float rc_min_vbv_overflow_use;
> > > > > +
> > > > > + /**
> > > > > + * qpfile to force frametypes or QPs for some or all frames,
> > only
> > > > used for libx264
> > > > > + * - encoding: Set by user.
> > > > > + * - decoding: unused.
> > > > > + */
> > > > > + char* qpfile;
> > > > > } AVCodecContext;
> > > >
> > > > lavc does not access files out of design principles ... (its something
> > > > about
> > > > the media file not being a file, there being no disk or such ...)
> > > >
> > > > also setting QPs should be supported over the input AVFrames
> > > > its the user apps job (=ffmpeg.c) to read and parse qpfile and
> > > > set the fields of the structs.
> > > >
> > > > now i see and agree this would be code duplication between user apps,
> > > > and libav could provide helper functions to do such common tasks but
> > that
> > > > has to be kept as optional part not as only and mandatory way to
> > > > perform something
> > > >
> > > > [...]
> > > > --
> > > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > > >
> > > > He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
> > > >
> > > > -----BEGIN PGP SIGNATURE-----
> > > > Version: GnuPG v1.4.9 (GNU/Linux)
> > > >
> > > > iD8DBQFJqAXvYR7HhwQLD6sRAmq8AJoDZoEZ9sVO2EYt9duJLe/zRUtf6gCgi8cH
> > > > kauojvUed2yrpvqu1A093so=
> > > > =nx3Y
> > > > -----END PGP SIGNATURE-----
> > > >
> > > > _______________________________________________
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel at mplayerhq.hu
> > > > https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
> > > >
> > >
> > > Michael,
> > >
> > > Thanks for comments. As I am new to ffmpeg, can you confirm my following
> > > questions?
> > >
> > > 1. I agree to put the option in the application level. If I do, I would
> > > parse the qpfile (with a helper function)
> > > against ost->frame_number at the beginning of
> > ffmpeg.c/do_video_output,
> > > and then set the fields of
> > > in_picture structure. I assume the in_picture.pict_type should be set
> > to
> > > frame_type from qpfile, but
> > > how to set qp value to in_picture?
> >
> > AVFrame.quality should work but it might not in which case that need fixing
> > ...
> > and there is rc_override
> >
> >
> > >
> > > 2. Where do you think is the best place to put the parse_qpfile helper
> > > function?
> >
> > a new libavcodec/helper_api.c/h
> >
> >
> > [...]
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > Many that live deserve death. And some that die deserve life. Can you give
> > it to them? Then do not be too eager to deal out death in judgement. For
> > even the very wise cannot see all ends. -- Gandalf
> >
> > -----BEGIN PGP SIGNATURE-----
> > Version: GnuPG v1.4.9 (GNU/Linux)
> >
> > iD8DBQFJqTiDYR7HhwQLD6sRAtOeAJoD1uHIyr5DRsUfDWGimU8GsKHY7gCfboMZ
> > GtvbP+DGqIuWE7Mi/HlukYc=
> > =PzP6
> > -----END PGP SIGNATURE-----
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at mplayerhq.hu
> > https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
> >
>
> Michael,
>
> How about this one? I am not quite sure about the code placement in
> ffmpeg.c but
> this version seems to work at least with libx264...
[...]
> @@ -419,6 +421,9 @@ static int av_exit(int ret)
> fclose(vstats_file);
> av_free(vstats_filename);
>
> + if (qpfile)
> + fclose(qpfile);
> +
trailing whitespace
[...]
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index b078bc9..16ccf9c 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3078,6 +3078,14 @@ void avcodec_default_free_buffers(AVCodecContext *s);
> char av_get_pict_type_char(int pict_type);
>
> /**
> + * Returns the picture type for the given the picture type char \p pict_type_char.
> + *
> + * @param[in] pict_type_char the picture type char
> + * @return The picture type.
> + */
> +int av_get_pict_type(char pict_type_char);
> +
> +/**
> * Returns codec bits per sample.
> *
> * @param[in] codec_id the codec
unrelated
> diff --git a/libavcodec/helper_api.c b/libavcodec/helper_api.c
> new file mode 100644
> index 0000000..7803ef7
> --- /dev/null
> +++ b/libavcodec/helper_api.c
adding of a new API should be a seperate patch
[...]
> +/**
> + * each line of QPfile is of this form:
> + * frame_number frame_type_char quality
> + * e.g: 80 P 20, 100 I 0
> + */
> +typedef struct QPEntry {
> + int frame_number;
> + int frame_type; /* Refer to get_pict_type_char */
> + int quality;
> +} QPEntry;
duplicate of RcOverride
[...]
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index d82756b..b369f06 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -85,7 +85,25 @@ X264_frame(AVCodecContext *ctx, uint8_t *buf, int bufsize, void *data)
> }
>
> x4->pic.i_pts = frame->pts;
> - x4->pic.i_type = X264_TYPE_AUTO;
> + switch (frame->pict_type) {
> + case FF_I_TYPE:
> + x4->pic.i_type = X264_TYPE_IDR;
> + break;
> + case FF_SI_TYPE:
> + x4->pic.i_type = X264_TYPE_I;
> + break;
> + case FF_P_TYPE:
> + x4->pic.i_type = X264_TYPE_P;
> + break;
> + case FF_B_TYPE:
> + x4->pic.i_type = X264_TYPE_BREF;
> + break;
> + case FF_BI_TYPE:
> + x4->pic.i_type = X264_TYPE_B;
> + break;
> + default:
> + x4->pic.i_type = X264_TYPE_AUTO;
> + }
> }
>
> if(x264_encoder_encode(x4->enc, &nal, &nnal, frame? &x4->pic: NULL,
seperate bugfix that belongs in a seperate patch
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090411/25378359/attachment.pgp>
More information about the ffmpeg-devel
mailing list