On Wed, Nov 11, 2015 at 4:53 AM, wm4 <nfx...@googlemail.com> wrote: > On Tue, 10 Nov 2015 22:42:31 +0100 > Michael Niedermayer <mich...@niedermayer.cc> wrote: > >> On Tue, Nov 10, 2015 at 03:15:50PM +0100, wm4 wrote: >> > On Tue, 10 Nov 2015 14:31:26 +0100 >> > Michael Niedermayer <mich...@niedermayer.cc> wrote: >> > >> > > On Tue, Nov 10, 2015 at 04:54:17PM +0700, Muhammad Faiz wrote: >> > > > On Mon, Nov 9, 2015 at 11:22 PM, wm4 <nfx...@googlemail.com> wrote: >> > > > > On Mon, 9 Nov 2015 08:03:54 -0800 >> > > > > Muhammad Faiz <mfc...@gmail.com> wrote: >> > > > > >> > > > >> From 4dcbda2e585404d2d79d5afcdc13fcb699f6f158 Mon Sep 17 00:00:00 >> > > > >> 2001 >> > > > >> From: Muhammad Faiz <mfc...@gmail.com> >> > > > >> Date: Mon, 9 Nov 2015 15:55:13 +0700 >> > > > >> Subject: [PATCH 1/2] avcodec/wrapped_avframe: implement >> > > > >> wrapped_avframe >> > > > >> decoder >> > > > >> >> > > > >> fix ticket #4985 >> > > > >> for use in avdevice/lavfi >> > > > >> --- >> > > > >> libavcodec/Makefile | 1 + >> > > > >> libavcodec/allcodecs.c | 2 +- >> > > > >> libavcodec/version.h | 2 +- >> > > > >> libavcodec/wrapped_avframe.c | 32 ++++++++++++++++++++++++++++++++ >> > > > >> 4 files changed, 35 insertions(+), 2 deletions(-) >> > > > >> >> > > > >> diff --git a/libavcodec/Makefile b/libavcodec/Makefile >> > > > >> index 68a573f..c60d512 100644 >> > > > >> --- a/libavcodec/Makefile >> > > > >> +++ b/libavcodec/Makefile >> > > > >> @@ -577,6 +577,7 @@ OBJS-$(CONFIG_WMV2_ENCODER) += >> > > > >> wmv2enc.o wmv2.o \ >> > > > >> msmpeg4.o msmpeg4enc.o >> > > > >> msmpeg4data.o >> > > > >> OBJS-$(CONFIG_WNV1_DECODER) += wnv1.o >> > > > >> OBJS-$(CONFIG_WS_SND1_DECODER) += ws-snd1.o >> > > > >> +OBJS-$(CONFIG_WRAPPED_AVFRAME_DECODER) += wrapped_avframe.o >> > > > >> OBJS-$(CONFIG_WRAPPED_AVFRAME_ENCODER) += wrapped_avframe.o >> > > > >> OBJS-$(CONFIG_XAN_DPCM_DECODER) += dpcm.o >> > > > >> OBJS-$(CONFIG_XAN_WC3_DECODER) += xan.o >> > > > >> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c >> > > > >> index 9f60d7c..3260927 100644 >> > > > >> --- a/libavcodec/allcodecs.c >> > > > >> +++ b/libavcodec/allcodecs.c >> > > > >> @@ -342,7 +342,7 @@ void avcodec_register_all(void) >> > > > >> REGISTER_DECODER(VP9, vp9); >> > > > >> REGISTER_DECODER(VQA, vqa); >> > > > >> REGISTER_DECODER(WEBP, webp); >> > > > >> - REGISTER_ENCODER(WRAPPED_AVFRAME, wrapped_avframe); >> > > > >> + REGISTER_ENCDEC (WRAPPED_AVFRAME, wrapped_avframe); >> > > > >> REGISTER_ENCDEC (WMV1, wmv1); >> > > > >> REGISTER_ENCDEC (WMV2, wmv2); >> > > > >> REGISTER_DECODER(WMV3, wmv3); >> > > > >> diff --git a/libavcodec/version.h b/libavcodec/version.h >> > > > >> index 1e21f15..5eecf5b 100644 >> > > > >> --- a/libavcodec/version.h >> > > > >> +++ b/libavcodec/version.h >> > > > >> @@ -29,7 +29,7 @@ >> > > > >> #include "libavutil/version.h" >> > > > >> >> > > > >> #define LIBAVCODEC_VERSION_MAJOR 57 >> > > > >> -#define LIBAVCODEC_VERSION_MINOR 15 >> > > > >> +#define LIBAVCODEC_VERSION_MINOR 16 >> > > > >> #define LIBAVCODEC_VERSION_MICRO 100 >> > > > >> >> > > > >> #define LIBAVCODEC_VERSION_INT >> > > > >> AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ >> > > > >> diff --git a/libavcodec/wrapped_avframe.c >> > > > >> b/libavcodec/wrapped_avframe.c >> > > > >> index 13c8d8a..185a1a2 100644 >> > > > >> --- a/libavcodec/wrapped_avframe.c >> > > > >> +++ b/libavcodec/wrapped_avframe.c >> > > > >> @@ -32,6 +32,8 @@ >> > > > >> #include "libavutil/buffer.h" >> > > > >> #include "libavutil/pixdesc.h" >> > > > >> >> > > > >> +#if CONFIG_WRAPPED_AVFRAME_ENCODER >> > > > >> + >> > > > >> static void wrapped_avframe_release_buffer(void *unused, uint8_t >> > > > >> *data) >> > > > >> { >> > > > >> AVFrame *frame = (AVFrame *)data; >> > > > >> @@ -71,3 +73,33 @@ AVCodec ff_wrapped_avframe_encoder = { >> > > > >> .encode2 = wrapped_avframe_encode, >> > > > >> .caps_internal = FF_CODEC_CAP_INIT_THREADSAFE, >> > > > >> }; >> > > > >> + >> > > > >> +#endif >> > > > >> + >> > > > >> +#if CONFIG_WRAPPED_AVFRAME_DECODER >> > > > >> + >> > > > >> +static int wrapped_avframe_decode(AVCodecContext *avctx, void >> > > > >> *data, >> > > > >> + int *got_frame, AVPacket *avpkt) >> > > > >> +{ >> > > > >> + int ret; >> > > > >> + >> > > > >> + if (avpkt->size != sizeof(AVFrame)) >> > > > >> + return AVERROR(EINVAL); >> > > > >> + >> > > > >> + if ((ret = av_frame_ref((AVFrame *) data, (AVFrame *) >> > > > >> avpkt->data)) < 0) >> > > > >> + return ret; >> > > > >> + >> > > > >> + *got_frame = 1; >> > > > >> + return avpkt->size; >> > > > >> +} >> > > > >> + >> > > > >> +AVCodec ff_wrapped_avframe_decoder = { >> > > > >> + .name = "wrapped_avframe", >> > > > >> + .long_name = NULL_IF_CONFIG_SMALL("AVFrame to AVPacket >> > > > >> passthrough"), >> > > > >> + .type = AVMEDIA_TYPE_VIDEO, >> > > > >> + .id = AV_CODEC_ID_WRAPPED_AVFRAME, >> > > > >> + .decode = wrapped_avframe_decode, >> > > > >> + .caps_internal = FF_CODEC_CAP_INIT_THREADSAFE, >> > > > >> +}; >> > > > >> + >> > > > >> +#endif >> > > > > >> > > > > This is very dangerous. You get potentially security relevant bad >> > > > > behavior if you get anyone to force a demuxer/decoder on an untrusted >> > > > > input file. >> > > > >> > > > OK, this makes segfault on my machine (120 = sizeof(AVFrame)/4 on my >> > > > machine) >> > > > ffmpeg -codec wrapped_avframe -pixel_format rgba -f rawvideo -s 120x1 >> > > > -i input.mkv -f null -y /dev/null >> > > >> > > maybe in addition to any other saftey features >> > > "-codec wrapped_avframe" could be checked for and forbidden in >> > > libavformat, or does this have any safe use ? >> > >> > You could argue that API users can expect that there will be no >> > security or stability issues even if they let an user decide with which >> > codec to initialize a AVCodecContext. >> > >> > I'm unsure to what extent we want to fulfill such expectations. >> >> it would be ideal if it was safe for the user to set any codec >> >> about the packet flags idea, that potentially might break user apps >> which do not pass unknown new flags between their demuxer and >> decoder layers. >> So if we do that, we must document it in APIChanges or similar >> >> dont really know what is best > > A side data could possibly be used to signal this as well. > > Anyway, surely someone has a better idea.
Packet flags is better and simpler if we ignore compatibility issue, side data is OK though more complicated, maybe it should be encapsulated using API (av_packet_frame_*) to hide the implementation and remove duplicate code. Not sure what should I do, but anyway thank's for the review _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel