From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 869684096E for ; Sun, 3 Jul 2022 13:19:16 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5580B68B9CB; Sun, 3 Jul 2022 16:19:14 +0300 (EEST) Received: from nef.ens.fr (nef2.ens.fr [129.199.96.40]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A182A68B947 for ; Sun, 3 Jul 2022 16:19:07 +0300 (EEST) X-ENS-nef-client: 129.199.129.80 ( name = phare.normalesup.org ) Received: from phare.normalesup.org (phare.normalesup.org [129.199.129.80]) by nef.ens.fr (8.14.4/1.01.28121999) with ESMTP id 263DJ6ek015327 ; Sun, 3 Jul 2022 15:19:06 +0200 Received: by phare.normalesup.org (Postfix, from userid 1001) id 9D6A4E00E9; Sun, 3 Jul 2022 15:19:06 +0200 (CEST) Date: Sun, 3 Jul 2022 15:19:06 +0200 From: Nicolas George To: FFmpeg development discussions and patches Message-ID: References: <20220629195250.29341-1-timo@rothenpieler.org> MIME-Version: 1.0 In-Reply-To: <20220629195250.29341-1-timo@rothenpieler.org> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (nef.ens.fr [129.199.96.32]); Sun, 03 Jul 2022 15:19:07 +0200 (CEST) Subject: Re: [FFmpeg-devel] [RFC] avdevice/lavfi: output wrapped AVFrames X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Timo Rothenpieler Content-Type: multipart/mixed; boundary="===============2607876319546260337==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============2607876319546260337== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="B8Xj/QpAcDaY1YAA" Content-Disposition: inline --B8Xj/QpAcDaY1YAA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Timo Rothenpieler (12022-06-29): > The lavfi avdevice as it is right now can't output "special frames" > like hardware frames. > This patch makes the lavfi avdevice output wrapped AVFrames instead > of copying the actual data, greatly simplifying it in the process. Thanks for the patch. I am not familiar with the hardware frame infrastructure, but this patch makes the whole code much simpler, and assumedly more efficient to boot. That makes it worth it for that reason alone. If it also allows to use hardware frames, even in a limited or fragile way, it is a bonus. I do not see how anybody could object. > I am not at all sure if this has some unexpected consequences. > It works just fine with ffmpeg.c, but it might be an ABI break for > potential library consumers? I think the case can be made for both positions: on one hand, an application should not assume a demuxer will output any kind of frame and should be ready for wrapped AVFrame; on the other hand, devices are special and somebody who uses a specific device on purpose may make assumptions on its behavior. I think this case is fringe enough that we can accept the break, at least if the device only outputs normal frames by default, not hardware frames. >=20 > libavdevice/lavfi.c | 108 +++++++++++++------------------------------- > 1 file changed, 31 insertions(+), 77 deletions(-) >=20 > diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c > index db5d0b94de..6bfdb5033e 100644 > --- a/libavdevice/lavfi.c > +++ b/libavdevice/lavfi.c > @@ -54,32 +54,10 @@ typedef struct { > int *sink_eof; > int *stream_sink_map; > int *sink_stream_subcc_map; > - AVFrame *decoded_frame; > int nb_sinks; > AVPacket subcc_packet; > } LavfiContext; > =20 > -static int *create_all_formats(int n) > -{ > - int i, j, *fmts, count =3D 0; > - > - for (i =3D 0; i < n; i++) { > - const AVPixFmtDescriptor *desc =3D av_pix_fmt_desc_get(i); > - if (!(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) > - count++; > - } > - > - if (!(fmts =3D av_malloc_array(count + 1, sizeof(*fmts)))) > - return NULL; > - for (j =3D 0, i =3D 0; i < n; i++) { > - const AVPixFmtDescriptor *desc =3D av_pix_fmt_desc_get(i); > - if (!(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) > - fmts[j++] =3D i; > - } > - fmts[j] =3D AV_PIX_FMT_NONE; > - return fmts; > -} > - > av_cold static int lavfi_read_close(AVFormatContext *avctx) > { > LavfiContext *lavfi =3D avctx->priv_data; > @@ -90,7 +68,6 @@ av_cold static int lavfi_read_close(AVFormatContext *av= ctx) > av_freep(&lavfi->sink_stream_subcc_map); > av_freep(&lavfi->sinks); > avfilter_graph_free(&lavfi->graph); > - av_frame_free(&lavfi->decoded_frame); > =20 > return 0; > } > @@ -125,15 +102,11 @@ av_cold static int lavfi_read_header(AVFormatContex= t *avctx) > LavfiContext *lavfi =3D avctx->priv_data; > AVFilterInOut *input_links =3D NULL, *output_links =3D NULL, *inout; > const AVFilter *buffersink, *abuffersink; > - int *pix_fmts =3D create_all_formats(AV_PIX_FMT_NB); > enum AVMediaType type; > int ret =3D 0, i, n; > =20 > #define FAIL(ERR) { ret =3D ERR; goto end; } > =20 > - if (!pix_fmts) > - FAIL(AVERROR(ENOMEM)); > - > buffersink =3D avfilter_get_by_name("buffersink"); > abuffersink =3D avfilter_get_by_name("abuffersink"); > =20 > @@ -264,22 +237,12 @@ av_cold static int lavfi_read_header(AVFormatContex= t *avctx) > ret =3D avfilter_graph_create_filter(&sink, buffersink, > inout->name, NULL, > NULL, lavfi->graph); > - if (ret >=3D 0) > - ret =3D av_opt_set_int_list(sink, "pix_fmts", pix_fmts, = AV_PIX_FMT_NONE, AV_OPT_SEARCH_CHILDREN); > if (ret < 0) > goto end; > } else if (type =3D=3D AVMEDIA_TYPE_AUDIO) { > - static const enum AVSampleFormat sample_fmts[] =3D { > - AV_SAMPLE_FMT_U8, AV_SAMPLE_FMT_S16, AV_SAMPLE_FMT_S32, > - AV_SAMPLE_FMT_FLT, AV_SAMPLE_FMT_DBL, > - }; > - > ret =3D avfilter_graph_create_filter(&sink, abuffersink, > inout->name, NULL, > NULL, lavfi->graph); > - if (ret >=3D 0) > - ret =3D av_opt_set_bin(sink, "sample_fmts", (const uint8= _t*)sample_fmts, > - sizeof(sample_fmts), AV_OPT_SEARCH_= CHILDREN); > if (ret < 0) > goto end; > ret =3D av_opt_set_int(sink, "all_channel_counts", 1, > @@ -320,39 +283,25 @@ av_cold static int lavfi_read_header(AVFormatContex= t *avctx) > AVCodecParameters *const par =3D st->codecpar; > avpriv_set_pts_info(st, 64, time_base.num, time_base.den); > par->codec_type =3D av_buffersink_get_type(sink); > + par->codec_id =3D AV_CODEC_ID_WRAPPED_AVFRAME; > + par->format =3D av_buffersink_get_format(sink); > if (par->codec_type =3D=3D AVMEDIA_TYPE_VIDEO) { > - int64_t probesize; > - par->codec_id =3D AV_CODEC_ID_RAWVIDEO; > - par->format =3D av_buffersink_get_format(sink); > par->width =3D av_buffersink_get_w(sink); > par->height =3D av_buffersink_get_h(sink); > - probesize =3D par->width * par->height * 30 * > - av_get_padded_bits_per_pixel(av_pix_fmt_de= sc_get(par->format)); > - avctx->probesize =3D FFMAX(avctx->probesize, probesize); > - st ->sample_aspect_ratio =3D > + st ->sample_aspect_ratio =3D > par->sample_aspect_ratio =3D av_buffersink_get_sample_aspect= _ratio(sink); > } else if (par->codec_type =3D=3D AVMEDIA_TYPE_AUDIO) { > par->sample_rate =3D av_buffersink_get_sample_rate(sink); > ret =3D av_buffersink_get_ch_layout(sink, &par->ch_layout); > if (ret < 0) > goto end; > - par->format =3D av_buffersink_get_format(sink); > - par->codec_id =3D av_get_pcm_codec(par->format, -1); > - if (par->codec_id =3D=3D AV_CODEC_ID_NONE) > - av_log(avctx, AV_LOG_ERROR, > - "Could not find PCM codec for sample format %s.\n= ", > - av_get_sample_fmt_name(par->format)); > } > } > =20 > if ((ret =3D create_subcc_streams(avctx)) < 0) > goto end; > =20 > - if (!(lavfi->decoded_frame =3D av_frame_alloc())) > - FAIL(AVERROR(ENOMEM)); > - > end: > - av_free(pix_fmts); > avfilter_inout_free(&input_links); > avfilter_inout_free(&output_links); > return ret; > @@ -378,22 +327,31 @@ static int create_subcc_packet(AVFormatContext *avc= tx, AVFrame *frame, > return 0; > } > =20 > +static void lavfi_free_frame(void *opaque, uint8_t *data) > +{ > + AVFrame *frame =3D (AVFrame*)data; > + av_frame_free(&frame); > +} > + > static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt) > { > LavfiContext *lavfi =3D avctx->priv_data; > double min_pts =3D DBL_MAX; > int stream_idx, min_pts_sink_idx =3D 0; > - AVFrame *frame =3D lavfi->decoded_frame; > + AVFrame *frame; > AVDictionary *frame_metadata; > int ret, i; > int size =3D 0; > - AVStream *st; > =20 > if (lavfi->subcc_packet.size) { > av_packet_move_ref(pkt, &lavfi->subcc_packet); > return pkt->size; > } > =20 > + frame =3D av_frame_alloc(); > + if (!frame) > + return AVERROR(ENOMEM); > + > /* iterate through all the graph sinks. Select the sink with the > * minimum PTS */ > for (i =3D 0; i < lavfi->nb_sinks; i++) { > @@ -411,7 +369,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, = AVPacket *pkt) > lavfi->sink_eof[i] =3D 1; > continue; > } else if (ret < 0) > - return ret; > + goto fail; > d =3D av_rescale_q_rnd(frame->pts, tb, AV_TIME_BASE_Q, AV_ROUND_= NEAR_INF|AV_ROUND_PASS_MINMAX); > ff_dlog(avctx, "sink_idx:%d time:%f\n", i, d); > av_frame_unref(frame); > @@ -421,29 +379,15 @@ static int lavfi_read_packet(AVFormatContext *avctx= , AVPacket *pkt) > min_pts_sink_idx =3D i; > } > } > - if (min_pts =3D=3D DBL_MAX) > - return AVERROR_EOF; > + if (min_pts =3D=3D DBL_MAX) { > + ret =3D AVERROR_EOF; > + goto fail; > + } > =20 > ff_dlog(avctx, "min_pts_sink_idx:%i\n", min_pts_sink_idx); > =20 > av_buffersink_get_frame_flags(lavfi->sinks[min_pts_sink_idx], frame,= 0); > stream_idx =3D lavfi->sink_stream_map[min_pts_sink_idx]; > - st =3D avctx->streams[stream_idx]; > - > - if (st->codecpar->codec_type =3D=3D AVMEDIA_TYPE_VIDEO) { > - size =3D av_image_get_buffer_size(frame->format, frame->width, f= rame->height, 1); > - if ((ret =3D av_new_packet(pkt, size)) < 0) > - goto fail; > - > - av_image_copy_to_buffer(pkt->data, size, (const uint8_t **)frame= ->data, frame->linesize, > - frame->format, frame->width, frame->heig= ht, 1); > - } else if (st->codecpar->codec_type =3D=3D AVMEDIA_TYPE_AUDIO) { > - size =3D frame->nb_samples * av_get_bytes_per_sample(frame->form= at) * > - frame->ch_layout.nb_channels; > - if ((ret =3D av_new_packet(pkt, size)) < 0) > - goto fail; > - memcpy(pkt->data, frame->data[0], size); > - } > =20 > frame_metadata =3D frame->metadata; > if (frame_metadata) { > @@ -465,13 +409,23 @@ static int lavfi_read_packet(AVFormatContext *avctx= , AVPacket *pkt) > goto fail; > } > =20 > + pkt->buf =3D av_buffer_create((uint8_t*)frame, sizeof(*frame), > + &lavfi_free_frame, NULL, 0); > + if (!pkt->buf) { > + ret =3D AVERROR(ENOMEM); > + goto fail; > + } This is using sizeof(AVFrame) outside of libavutil, it is not allowed. I was about to suggest you make it a separate utility function, but it already exists: wrapped_avframe_encode(); please use it instead. (OTOH, wrapped_avframe_encode() also uses sizeof(AVFrame), it needs fixing.) > + > + pkt->data =3D pkt->buf->data; > + pkt->size =3D pkt->buf->size; > + pkt->flags |=3D AV_PKT_FLAG_TRUSTED; > + > pkt->stream_index =3D stream_idx; > pkt->pts =3D frame->pts; > pkt->pos =3D frame->pkt_pos; > - av_frame_unref(frame); > return size; > fail: > - av_frame_unref(frame); > + av_frame_free(&frame); > return ret; > =20 > } Regards, --=20 Nicolas George --B8Xj/QpAcDaY1YAA Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE6ooRQGBoNzw0KnwPcZVLI8pNxgwFAmLBl0gACgkQcZVLI8pN xgyS8hAAoOniJWRzj/7Iaz4nm/Ax7eNv34H9XwMXicy0YWdyPDHqXmVS1zVWDANu mci0jxXg5HITZHlqW5jfxo6uMjAHp20/A5G+0u8vJuq5AfH+0xtbJzDSMoDrXaYt QVmut9VMFdaeQICjr8erpC7K/qQUP4+zkPrUPUE0bm12SU0fkTu9ANo4yATXTbtm G0hFRyDwqcfgmTbYdW4dkt/+3zrqnhciQBX84gGqCD1xZwWvWQpnXetOTFKUx3bb crIwPtxuJRWSDt/6MrunpGZgjXj0LIsL+HpgvJTmLpcESoWF/LUPehcpxdOLNnwc sSLdAwqOK2UaJCiloqDmfE+yvScR3LbGPvBQt8vq27S3g4MuQxkvlWtjz69jGiH7 6CL37nAfILgYxes8ffKQbUzXH8w2B0DF4+YbFfKJbDUD+addI3fTt3HIOsjZrLn9 6RChTFMZl8n90xd53qHHzAeTv0C5XRn307uKu+sPdK4L7dQVCzEkf7pbYHhDmxaq RUURcUB6PH0T3SDM4GmI6o9L/Jpr+RfY4TqOdFwC6wo+DSX5IbC5qOxRLBSY/2LO GUqYarjz0YxtOKaNYeZ+XcWSW1Zyr1yDllA1BEPBmAZDzdA8oX8fyR1UMfnRVAmC xoYLOBILzu3EYLEbjepHzTcw/AHDXVQIP5k6EwtRIa6Oyt9K1VY= =JQoC -----END PGP SIGNATURE----- --B8Xj/QpAcDaY1YAA-- --===============2607876319546260337== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". --===============2607876319546260337==--