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...@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
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
