> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Nicolas George
> Sent: Friday, July 21, 2017 3:52 PM
> To: FFmpeg development discussions and patches
> <ffmpeg-devel@ffmpeg.org>
> Cc: Li, Zhong <zhong...@intel.com>; s...@jkqxz.net; Zhao, Jun
> <jun.z...@intel.com>; nfx...@googlemail.com
> Subject: Re: [FFmpeg-devel] [PATCH] doc/examples/encode_video: add qsv
> encoder support
>
> Le tridi 3 thermidor, an CCXXV, Zhong Li a écrit :
> > Subject: Re: [FFmpeg-devel] [PATCH] doc/examples/encode_video: add qsv
> > encoder support
>
> I do not think it is a good idea. Examples are meant to be simple. I think a
> separate example for hwdevice encoding would be a better idea, although it
> has drawbacks of its own (code duplication in the utility parts).
Ok, will rework to a separate qsv encoder example.
>
> > Signed-off-by: Zhong Li <zhong...@intel.com>
> > ---
> > doc/examples/encode_video.c | 32
> +++++++++++++++++++++++++++++---
> > 1 file changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/doc/examples/encode_video.c
> b/doc/examples/encode_video.c
> > index 8cd1321..9c26f63 100644
> > --- a/doc/examples/encode_video.c
> > +++ b/doc/examples/encode_video.c
> > @@ -35,6 +35,8 @@
> >
> > #include <libavutil/opt.h>
> > #include <libavutil/imgutils.h>
> > +#include "libavutil/buffer.h"
> > +#include "libavutil/hwcontext.h"
> >
> > static void encode(AVCodecContext *enc_ctx, AVFrame *frame,
> AVPacket *pkt,
> > FILE *outfile)
> > @@ -75,7 +77,10 @@ int main(int argc, char **argv)
> > FILE *f;
> > AVFrame *frame;
> > AVPacket *pkt;
>
> > + AVBufferRef* encode_device = NULL;
>
> Pointer marks belong with the variable, not the type.
>
> > uint8_t endcode[] = { 0, 0, 1, 0xb7 };
> > + enum AVHWDeviceType hw_device_type =
> AV_HWDEVICE_TYPE_NONE;
> > + enum AVPixelFormat pixel_format = AV_PIX_FMT_YUV420P;
> >
> > if (argc <= 2) {
> > fprintf(stderr, "Usage: %s <output file> <codec name>\n",
> > argv[0]); @@ -86,6 +91,21 @@ int main(int argc, char **argv)
> >
> > avcodec_register_all();
> >
>
> > + if (strstr(codec_name, "qsv")) {
>
> If this is necessary, then someone seriously messed up the API design.
> Fortunately, I see no trace of it in ffmpeg*.c, so I guess the correct way of
> detecting hwdevice encoding is not that.
It is similar to hw_device_match_type_in_name() in ffmpeg_hw.c:
if (strstr(codec_name, type_name))
return type;
>
> > + hw_device_type = AV_HWDEVICE_TYPE_QSV;
> > + pixel_format = AV_PIX_FMT_NV12;
> > + }
> > +
> > + /* open the hardware device */
> > + if (hw_device_type != AV_HWDEVICE_TYPE_NONE) {
>
> > + ret = av_hwdevice_ctx_create(&encode_device,
> hw_device_type,
> > + NULL, NULL, 0);
>
> I see that encode_device is only used later for freeing. Can you explain in a
> comment why it is necessary? Otherwise, user may think it is unnecessary
> and remove it.
The encode_device is passed to av_hwdevice_ctx_create to be written, it is
necessary for API requirement.
>
> > + if (ret < 0) {
> > + fprintf(stderr, "Cannot open the hardware device\n");
> > + exit(1);
> > + }
> > + }
> > +
> > /* find the mpeg1video encoder */
> > codec = avcodec_find_encoder_by_name(codec_name);
> > if (!codec) {
> > @@ -120,7 +140,7 @@ int main(int argc, char **argv)
> > */
> > c->gop_size = 10;
> > c->max_b_frames = 1;
> > - c->pix_fmt = AV_PIX_FMT_YUV420P;
> > + c->pix_fmt = pixel_format;
> >
> > if (codec->id == AV_CODEC_ID_H264)
> > av_opt_set(c->priv_data, "preset", "slow", 0); @@ -173,8
> > +193,13 @@ int main(int argc, char **argv)
> > /* Cb and Cr */
> > for (y = 0; y < c->height/2; y++) {
> > for (x = 0; x < c->width/2; x++) {
> > - frame->data[1][y * frame->linesize[1] + x] = 128 + y + i
> * 2;
> > - frame->data[2][y * frame->linesize[2] + x] = 64 + x + i
> * 5;
> > + if (frame->format == AV_PIX_FMT_YUV420P) {
> > + frame->data[1][y * frame->linesize[1] + x] = 128
> + y + i * 2;
> > + frame->data[2][y * frame->linesize[2] + x] = 64 +
> x + i * 5;
> > + } else if (frame->format == AV_PIX_FMT_NV12) {
> > + frame->data[1][y * frame->linesize[1] + 2 * x] =
> 128 + y + i * 2;
> > + frame->data[1][y * frame->linesize[1] + 2 * x + 1]
> = 64 + x + i * 5;
> > + }
> > }
> > }
> >
> > @@ -194,6 +219,7 @@ int main(int argc, char **argv)
> > avcodec_free_context(&c);
> > av_frame_free(&frame);
> > av_packet_free(&pkt);
> > + av_buffer_unref(&encode_device);
> >
> > return 0;
> > }
>
> Regards,
>
> --
> Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
