[FFmpeg-devel] [PATCH] doc/examples/encode_video: add qsv encoder support

[Nicolas George]

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).

> Signed-off-by: Zhong Li <zhong.li at 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.

> +        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.

> +        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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170721/19246b90/attachment.sig>