| Message ID | 20161023221200.GA18633@hocevar.net |
|---|---|
| State | Accepted |
| Commit | 3115550abe96de674dac42f02a0b464e137bfc20 |
| Headers | show |
On Mon, Oct 24, 2016, Sam Hocevar wrote: > This fixes data corruption issues in codecs that keep references on > one or several previous frames. This doesn't seem to have gotten much attention. Any comments maybe?
On Mon, 24 Oct 2016 00:12:00 +0200 Sam Hocevar <sam@hocevar.net> wrote: > This patch moves the av_frame_make_writable() call from fill_yuv_image > to get_video_frame so that its argument can be the actual frame that > will be sent to the encoder. > > This fixes data corruption issues in codecs that keep references on > one or several previous frames. > > Signed-off-by: Sam Hocevar <sam@hocevar.net> > --- > doc/examples/muxing.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/doc/examples/muxing.c b/doc/examples/muxing.c > index f1f5bb8..1df5912 100644 > --- a/doc/examples/muxing.c > +++ b/doc/examples/muxing.c > @@ -440,15 +440,7 @@ static void open_video(AVFormatContext *oc, AVCodec *codec, OutputStream *ost, A > static void fill_yuv_image(AVFrame *pict, int frame_index, > int width, int height) > { > - int x, y, i, ret; > - > - /* when we pass a frame to the encoder, it may keep a reference to it > - * internally; > - * make sure we do not overwrite it here > - */ > - ret = av_frame_make_writable(pict); > - if (ret < 0) > - exit(1); > + int x, y, i; > > i = frame_index; > > @@ -475,6 +467,11 @@ static AVFrame *get_video_frame(OutputStream *ost) > STREAM_DURATION, (AVRational){ 1, 1 }) >= 0) > return NULL; > > + /* when we pass a frame to the encoder, it may keep a reference to it > + * internally; make sure we do not overwrite it here */ > + if (av_frame_make_writable(ost->frame) < 0) > + exit(1); > + > if (c->pix_fmt != AV_PIX_FMT_YUV420P) { > /* as we only generate a YUV420P picture, we must convert it > * to the codec pixel format if needed */ LGTM, but I'm not in a position to push now.
On Wed, Nov 23, 2016 at 02:09:14AM +0100, wm4 wrote: > On Mon, 24 Oct 2016 00:12:00 +0200 > Sam Hocevar <sam@hocevar.net> wrote: > > > This patch moves the av_frame_make_writable() call from fill_yuv_image > > to get_video_frame so that its argument can be the actual frame that > > will be sent to the encoder. > > > > This fixes data corruption issues in codecs that keep references on > > one or several previous frames. > > > > Signed-off-by: Sam Hocevar <sam@hocevar.net> > > --- > > doc/examples/muxing.c | 15 ++++++--------- > > 1 file changed, 6 insertions(+), 9 deletions(-) > > > > diff --git a/doc/examples/muxing.c b/doc/examples/muxing.c > > index f1f5bb8..1df5912 100644 > > --- a/doc/examples/muxing.c > > +++ b/doc/examples/muxing.c > > @@ -440,15 +440,7 @@ static void open_video(AVFormatContext *oc, AVCodec *codec, OutputStream *ost, A > > static void fill_yuv_image(AVFrame *pict, int frame_index, > > int width, int height) > > { > > - int x, y, i, ret; > > - > > - /* when we pass a frame to the encoder, it may keep a reference to it > > - * internally; > > - * make sure we do not overwrite it here > > - */ > > - ret = av_frame_make_writable(pict); > > - if (ret < 0) > > - exit(1); > > + int x, y, i; > > > > i = frame_index; > > > > @@ -475,6 +467,11 @@ static AVFrame *get_video_frame(OutputStream *ost) > > STREAM_DURATION, (AVRational){ 1, 1 }) >= 0) > > return NULL; > > > > + /* when we pass a frame to the encoder, it may keep a reference to it > > + * internally; make sure we do not overwrite it here */ > > + if (av_frame_make_writable(ost->frame) < 0) > > + exit(1); > > + > > if (c->pix_fmt != AV_PIX_FMT_YUV420P) { > > /* as we only generate a YUV420P picture, we must convert it > > * to the codec pixel format if needed */ > > LGTM, but I'm not in a position to push now. applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato
diff --git a/doc/examples/muxing.c b/doc/examples/muxing.c index f1f5bb8..1df5912 100644 --- a/doc/examples/muxing.c +++ b/doc/examples/muxing.c @@ -440,15 +440,7 @@ static void open_video(AVFormatContext *oc, AVCodec *codec, OutputStream *ost, A static void fill_yuv_image(AVFrame *pict, int frame_index, int width, int height) { - int x, y, i, ret; - - /* when we pass a frame to the encoder, it may keep a reference to it - * internally; - * make sure we do not overwrite it here - */ - ret = av_frame_make_writable(pict); - if (ret < 0) - exit(1); + int x, y, i; i = frame_index; @@ -475,6 +467,11 @@ static AVFrame *get_video_frame(OutputStream *ost) STREAM_DURATION, (AVRational){ 1, 1 }) >= 0) return NULL; + /* when we pass a frame to the encoder, it may keep a reference to it + * internally; make sure we do not overwrite it here */ + if (av_frame_make_writable(ost->frame) < 0) + exit(1); + if (c->pix_fmt != AV_PIX_FMT_YUV420P) { /* as we only generate a YUV420P picture, we must convert it * to the codec pixel format if needed */
This patch moves the av_frame_make_writable() call from fill_yuv_image to get_video_frame so that its argument can be the actual frame that will be sent to the encoder. This fixes data corruption issues in codecs that keep references on one or several previous frames. Signed-off-by: Sam Hocevar <sam@hocevar.net> --- doc/examples/muxing.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)