| Message ID | 20190826161727.1255-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Aug 26, 2019 at 01:17:25PM -0300, James Almer wrote: > Used to signal frames that can be safely discarded without losing > any picture data, side data, or metadata other than timing info. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > This implements the "disposable frame" solution to allow library > users to drop duplicate frames before further processing if desired, > instead of forcing decoders to output vfr content when cfr is coded > in the bitstream. > > doc/APIchanges | 3 +++ > libavutil/frame.h | 5 +++++ > libavutil/version.h | 2 +- > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 682b67aa25..b28d702bae 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h > + Add AV_FRAME_FLAG_DISPOSABLE > + > 2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h > 4K limit removed from avio_printf. > > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 5d3231e7bb..e1bf8795d2 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -522,6 +522,11 @@ typedef struct AVFrame { > * A flag to mark the frames which need to be decoded, but shouldn't be output. > */ > #define AV_FRAME_FLAG_DISCARD (1 << 2) > +/** > + * A flag to indicate frames that can be discarded by the encoder. I.e. frames > + * that are an exact duplicate of the previous one. > + */ ... exact duplicate of the previous one, except its timestamp and duration. maybe AV_FRAME_FLAG_DUPLICATE or AV_FRAME_FLAG_REPEATED would be clearer ? thx [...]
On 8/26/2019 5:20 PM, Michael Niedermayer wrote: > On Mon, Aug 26, 2019 at 01:17:25PM -0300, James Almer wrote: >> Used to signal frames that can be safely discarded without losing >> any picture data, side data, or metadata other than timing info. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> This implements the "disposable frame" solution to allow library >> users to drop duplicate frames before further processing if desired, >> instead of forcing decoders to output vfr content when cfr is coded >> in the bitstream. >> >> doc/APIchanges | 3 +++ >> libavutil/frame.h | 5 +++++ >> libavutil/version.h | 2 +- >> 3 files changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/doc/APIchanges b/doc/APIchanges >> index 682b67aa25..b28d702bae 100644 >> --- a/doc/APIchanges >> +++ b/doc/APIchanges >> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 >> >> API changes, most recent first: >> >> +2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h >> + Add AV_FRAME_FLAG_DISPOSABLE >> + >> 2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h >> 4K limit removed from avio_printf. >> >> diff --git a/libavutil/frame.h b/libavutil/frame.h >> index 5d3231e7bb..e1bf8795d2 100644 >> --- a/libavutil/frame.h >> +++ b/libavutil/frame.h >> @@ -522,6 +522,11 @@ typedef struct AVFrame { >> * A flag to mark the frames which need to be decoded, but shouldn't be output. >> */ >> #define AV_FRAME_FLAG_DISCARD (1 << 2) >> +/** >> + * A flag to indicate frames that can be discarded by the encoder. I.e. frames >> + * that are an exact duplicate of the previous one. >> + */ > > ... exact duplicate of the previous one, except its timestamp and duration. > > maybe AV_FRAME_FLAG_DUPLICATE or AV_FRAME_FLAG_REPEATED would be clearer ? It might, but i wanted to use the same name as the AVPacket flag defined in avcodec.h. If duplicate or repeated is preferred then i'll change it. I could also instead change the doxy to state it signals that the frame is disposable in general and not just because it's a duplicate of a previous one, even if for now that's the only usecase for it. > > thx > > [...] > > > _______________________________________________ > 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". >
On Mon, Aug 26, 2019 at 05:23:22PM -0300, James Almer wrote: > On 8/26/2019 5:20 PM, Michael Niedermayer wrote: > > On Mon, Aug 26, 2019 at 01:17:25PM -0300, James Almer wrote: > >> Used to signal frames that can be safely discarded without losing > >> any picture data, side data, or metadata other than timing info. > >> > >> Signed-off-by: James Almer <jamrial@gmail.com> > >> --- > >> This implements the "disposable frame" solution to allow library > >> users to drop duplicate frames before further processing if desired, > >> instead of forcing decoders to output vfr content when cfr is coded > >> in the bitstream. > >> > >> doc/APIchanges | 3 +++ > >> libavutil/frame.h | 5 +++++ > >> libavutil/version.h | 2 +- > >> 3 files changed, 9 insertions(+), 1 deletion(-) > >> > >> diff --git a/doc/APIchanges b/doc/APIchanges > >> index 682b67aa25..b28d702bae 100644 > >> --- a/doc/APIchanges > >> +++ b/doc/APIchanges > >> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >> > >> API changes, most recent first: > >> > >> +2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h > >> + Add AV_FRAME_FLAG_DISPOSABLE > >> + > >> 2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h > >> 4K limit removed from avio_printf. > >> > >> diff --git a/libavutil/frame.h b/libavutil/frame.h > >> index 5d3231e7bb..e1bf8795d2 100644 > >> --- a/libavutil/frame.h > >> +++ b/libavutil/frame.h > >> @@ -522,6 +522,11 @@ typedef struct AVFrame { > >> * A flag to mark the frames which need to be decoded, but shouldn't be output. > >> */ > >> #define AV_FRAME_FLAG_DISCARD (1 << 2) > >> +/** > >> + * A flag to indicate frames that can be discarded by the encoder. I.e. frames > >> + * that are an exact duplicate of the previous one. > >> + */ > > > > ... exact duplicate of the previous one, except its timestamp and duration. > > > > maybe AV_FRAME_FLAG_DUPLICATE or AV_FRAME_FLAG_REPEATED would be clearer ? > > It might, but i wanted to use the same name as the AVPacket flag defined > in avcodec.h. If duplicate or repeated is preferred then i'll change it. wouldnt this overload the meaning of "discard" ? where the AVPacket flag does not neccesarily indicate identical data but iam fine with any name really, the other names was just a thought that came to my mind when reading the patch ... thx [...]
On 8/26/2019 5:30 PM, Michael Niedermayer wrote: > On Mon, Aug 26, 2019 at 05:23:22PM -0300, James Almer wrote: >> On 8/26/2019 5:20 PM, Michael Niedermayer wrote: >>> On Mon, Aug 26, 2019 at 01:17:25PM -0300, James Almer wrote: >>>> Used to signal frames that can be safely discarded without losing >>>> any picture data, side data, or metadata other than timing info. >>>> >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> This implements the "disposable frame" solution to allow library >>>> users to drop duplicate frames before further processing if desired, >>>> instead of forcing decoders to output vfr content when cfr is coded >>>> in the bitstream. >>>> >>>> doc/APIchanges | 3 +++ >>>> libavutil/frame.h | 5 +++++ >>>> libavutil/version.h | 2 +- >>>> 3 files changed, 9 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/doc/APIchanges b/doc/APIchanges >>>> index 682b67aa25..b28d702bae 100644 >>>> --- a/doc/APIchanges >>>> +++ b/doc/APIchanges >>>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 >>>> >>>> API changes, most recent first: >>>> >>>> +2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h >>>> + Add AV_FRAME_FLAG_DISPOSABLE >>>> + >>>> 2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h >>>> 4K limit removed from avio_printf. >>>> >>>> diff --git a/libavutil/frame.h b/libavutil/frame.h >>>> index 5d3231e7bb..e1bf8795d2 100644 >>>> --- a/libavutil/frame.h >>>> +++ b/libavutil/frame.h >>>> @@ -522,6 +522,11 @@ typedef struct AVFrame { >>>> * A flag to mark the frames which need to be decoded, but shouldn't be output. >>>> */ >>>> #define AV_FRAME_FLAG_DISCARD (1 << 2) >>>> +/** >>>> + * A flag to indicate frames that can be discarded by the encoder. I.e. frames >>>> + * that are an exact duplicate of the previous one. >>>> + */ >>> >>> ... exact duplicate of the previous one, except its timestamp and duration. >>> >>> maybe AV_FRAME_FLAG_DUPLICATE or AV_FRAME_FLAG_REPEATED would be clearer ? >> >> It might, but i wanted to use the same name as the AVPacket flag defined >> in avcodec.h. If duplicate or repeated is preferred then i'll change it. > > wouldnt this overload the meaning of "discard" ? > where the AVPacket flag does not neccesarily indicate identical data > but iam fine with any name really, the other names was just a thought that > came to my mind when reading the patch ... AV_PKT_FLAG_DISPOSABLE is currently used to signal packets with encoded frames a decoder can safely drop without processing. Right now it's being used only for non-ref B frames, but the doxy allows it to be implemented for other use cases as well. With AV_FRAME_FLAG_DISPOSABLE, the idea would be signaling a frame that can in theory be dropped without affecting whatever the encoder, filter or player handling them will output. So ideally, both "disposable" flags, frame and packet, would be defined in a generic enough way to essentially mean "a frame/packet that can be dropped without putting the ongoing process at risk". If we instead use "duplicate" or "repeated" here, then we're defining a much bigger constrain regarding what the flag can be used for. And both "discard" flags are currently pretty much internal flags used by lavc's generic code to drop a frame after being decoded, ensuring it's never output. > > thx > > [...] > > > _______________________________________________ > 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". >
On Mon, 26 Aug 2019, James Almer wrote: > Used to signal frames that can be safely discarded without losing > any picture data, side data, or metadata other than timing info. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > This implements the "disposable frame" solution to allow library > users to drop duplicate frames before further processing if desired, > instead of forcing decoders to output vfr content when cfr is coded > in the bitstream. > > doc/APIchanges | 3 +++ > libavutil/frame.h | 5 +++++ > libavutil/version.h | 2 +- > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 682b67aa25..b28d702bae 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h > + Add AV_FRAME_FLAG_DISPOSABLE > + > 2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h > 4K limit removed from avio_printf. > > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 5d3231e7bb..e1bf8795d2 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -522,6 +522,11 @@ typedef struct AVFrame { > * A flag to mark the frames which need to be decoded, but shouldn't be output. > */ > #define AV_FRAME_FLAG_DISCARD (1 << 2) > +/** > + * A flag to indicate frames that can be discarded by the encoder. I.e. frames > + * that are an exact duplicate of the previous one. > + */ > +#define AV_FRAME_FLAG_DISPOSABLE (1 << 3) Such a flag has the problem that it is not really an attribute of a frame in itself, but of a frame of the frame chain. That can cause various issues: If the first frame is dropped, the next frame no longer becomes disposable. For the issue we are trying to solve I feel that it is better if we create a video filter which checks the buffers (and side data and crop parameters) if they are the same and drops the frame. It also allows more granular control (or setting parameters, like making sure you get a frame every 1 second or so, which is common requirement even for VFR), plus it can work for existing cases without this flag implemented. Regards, Marton
On Tue, Aug 27, 2019 at 09:12:53AM +0200, Marton Balint wrote: > > > On Mon, 26 Aug 2019, James Almer wrote: > > >Used to signal frames that can be safely discarded without losing > >any picture data, side data, or metadata other than timing info. > > > >Signed-off-by: James Almer <jamrial@gmail.com> > >--- > >This implements the "disposable frame" solution to allow library > >users to drop duplicate frames before further processing if desired, > >instead of forcing decoders to output vfr content when cfr is coded > >in the bitstream. > > > >doc/APIchanges | 3 +++ > >libavutil/frame.h | 5 +++++ > >libavutil/version.h | 2 +- > >3 files changed, 9 insertions(+), 1 deletion(-) > > > >diff --git a/doc/APIchanges b/doc/APIchanges > >index 682b67aa25..b28d702bae 100644 > >--- a/doc/APIchanges > >+++ b/doc/APIchanges > >@@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > > >API changes, most recent first: > > > >+2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h > >+ Add AV_FRAME_FLAG_DISPOSABLE > >+ > >2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h > > 4K limit removed from avio_printf. > > > >diff --git a/libavutil/frame.h b/libavutil/frame.h > >index 5d3231e7bb..e1bf8795d2 100644 > >--- a/libavutil/frame.h > >+++ b/libavutil/frame.h > >@@ -522,6 +522,11 @@ typedef struct AVFrame { > > * A flag to mark the frames which need to be decoded, but shouldn't be output. > > */ > >#define AV_FRAME_FLAG_DISCARD (1 << 2) > >+/** > >+ * A flag to indicate frames that can be discarded by the encoder. I.e. frames > >+ * that are an exact duplicate of the previous one. > >+ */ > >+#define AV_FRAME_FLAG_DISPOSABLE (1 << 3) > > Such a flag has the problem that it is not really an attribute of a frame in > itself, but of a frame of the frame chain. That can cause various issues: If > the first frame is dropped, the next frame no longer becomes disposable. > > For the issue we are trying to solve I feel that it is better if we create a > video filter which checks the buffers (and side data and crop parameters) if > they are the same and drops the frame. It also allows more granular control > (or setting parameters, like making sure you get a frame every 1 second or > so, which is common requirement even for VFR), plus it can work for existing > cases without this flag implemented. If that path is taken then probably a public function which checks if 2 AVFrames are using identical references could be factored out here, it might be usefull elsewhere. It also would need to be updated if more references counted fields are added Thanks [...]
On Tue, Aug 27, 2019 at 09:12:53AM +0200, Marton Balint wrote: > > > On Mon, 26 Aug 2019, James Almer wrote: > > >Used to signal frames that can be safely discarded without losing > >any picture data, side data, or metadata other than timing info. > > > >Signed-off-by: James Almer <jamrial@gmail.com> > >--- > >This implements the "disposable frame" solution to allow library > >users to drop duplicate frames before further processing if desired, > >instead of forcing decoders to output vfr content when cfr is coded > >in the bitstream. > > > >doc/APIchanges | 3 +++ > >libavutil/frame.h | 5 +++++ > >libavutil/version.h | 2 +- > >3 files changed, 9 insertions(+), 1 deletion(-) > > > >diff --git a/doc/APIchanges b/doc/APIchanges > >index 682b67aa25..b28d702bae 100644 > >--- a/doc/APIchanges > >+++ b/doc/APIchanges > >@@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > > >API changes, most recent first: > > > >+2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h > >+ Add AV_FRAME_FLAG_DISPOSABLE > >+ > >2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h > > 4K limit removed from avio_printf. > > > >diff --git a/libavutil/frame.h b/libavutil/frame.h > >index 5d3231e7bb..e1bf8795d2 100644 > >--- a/libavutil/frame.h > >+++ b/libavutil/frame.h > >@@ -522,6 +522,11 @@ typedef struct AVFrame { > > * A flag to mark the frames which need to be decoded, but shouldn't be output. > > */ > >#define AV_FRAME_FLAG_DISCARD (1 << 2) > >+/** > >+ * A flag to indicate frames that can be discarded by the encoder. I.e. frames > >+ * that are an exact duplicate of the previous one. > >+ */ > >+#define AV_FRAME_FLAG_DISPOSABLE (1 << 3) > > Such a flag has the problem that it is not really an attribute of a frame in > itself, but of a frame of the frame chain. That can cause various issues: If > the first frame is dropped, the next frame no longer becomes disposable. > > For the issue we are trying to solve I feel that it is better if we create a > video filter which checks the buffers (and side data and crop parameters) if > they are the same and drops the frame. It also allows more granular control > (or setting parameters, like making sure you get a frame every 1 second or > so, which is common requirement even for VFR), plus it can work for existing > cases without this flag implemented. One disadvantage of this with the current ref counting system is that in codecs which do in place changes in a buffer. Keeping a reference to a frame means the decoder must copy the frame to get a new writable one to do changes in. Not keeping a ref avoids this. A method which detects duplicates without keeping a +1 reference on the previous frame would be better in this respect ... Thanks [...]
On Tue, 27 Aug 2019, Michael Niedermayer wrote: > On Tue, Aug 27, 2019 at 09:12:53AM +0200, Marton Balint wrote: >> >> >> On Mon, 26 Aug 2019, James Almer wrote: >> >>> Used to signal frames that can be safely discarded without losing >>> any picture data, side data, or metadata other than timing info. >>> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> This implements the "disposable frame" solution to allow library >>> users to drop duplicate frames before further processing if desired, >>> instead of forcing decoders to output vfr content when cfr is coded >>> in the bitstream. >>> >>> doc/APIchanges | 3 +++ >>> libavutil/frame.h | 5 +++++ >>> libavutil/version.h | 2 +- >>> 3 files changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/doc/APIchanges b/doc/APIchanges >>> index 682b67aa25..b28d702bae 100644 >>> --- a/doc/APIchanges >>> +++ b/doc/APIchanges >>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 >>> >>> API changes, most recent first: >>> >>> +2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h >>> + Add AV_FRAME_FLAG_DISPOSABLE >>> + >>> 2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h >>> 4K limit removed from avio_printf. >>> >>> diff --git a/libavutil/frame.h b/libavutil/frame.h >>> index 5d3231e7bb..e1bf8795d2 100644 >>> --- a/libavutil/frame.h >>> +++ b/libavutil/frame.h >>> @@ -522,6 +522,11 @@ typedef struct AVFrame { >>> * A flag to mark the frames which need to be decoded, but shouldn't be output. >>> */ >>> #define AV_FRAME_FLAG_DISCARD (1 << 2) >>> +/** >>> + * A flag to indicate frames that can be discarded by the encoder. I.e. frames >>> + * that are an exact duplicate of the previous one. >>> + */ >>> +#define AV_FRAME_FLAG_DISPOSABLE (1 << 3) >> >> Such a flag has the problem that it is not really an attribute of a frame in >> itself, but of a frame of the frame chain. That can cause various issues: If >> the first frame is dropped, the next frame no longer becomes disposable. >> > >> For the issue we are trying to solve I feel that it is better if we create a >> video filter which checks the buffers (and side data and crop parameters) if >> they are the same and drops the frame. It also allows more granular control >> (or setting parameters, like making sure you get a frame every 1 second or >> so, which is common requirement even for VFR), plus it can work for existing >> cases without this flag implemented. > > One disadvantage of this with the current ref counting system is that in codecs > which do in place changes in a buffer. > Keeping a reference to a frame means the decoder must copy the frame to get a > new writable one to do changes in. Not keeping a ref avoids this. > A method which detects duplicates without keeping a +1 reference on the > previous frame would be better in this respect ... > That is a fair point. I am OK with the frame flags then. Thanks, Marton
On Tue, Aug 27, 2019 at 10:58:13AM +0200, Marton Balint wrote: > > > On Tue, 27 Aug 2019, Michael Niedermayer wrote: > > >On Tue, Aug 27, 2019 at 09:12:53AM +0200, Marton Balint wrote: > >> > >> > >>On Mon, 26 Aug 2019, James Almer wrote: > >> > >>>Used to signal frames that can be safely discarded without losing > >>>any picture data, side data, or metadata other than timing info. > >>> > >>>Signed-off-by: James Almer <jamrial@gmail.com> > >>>--- > >>>This implements the "disposable frame" solution to allow library > >>>users to drop duplicate frames before further processing if desired, > >>>instead of forcing decoders to output vfr content when cfr is coded > >>>in the bitstream. > >>> > >>>doc/APIchanges | 3 +++ > >>>libavutil/frame.h | 5 +++++ > >>>libavutil/version.h | 2 +- > >>>3 files changed, 9 insertions(+), 1 deletion(-) > >>> > >>>diff --git a/doc/APIchanges b/doc/APIchanges > >>>index 682b67aa25..b28d702bae 100644 > >>>--- a/doc/APIchanges > >>>+++ b/doc/APIchanges > >>>@@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >>> > >>>API changes, most recent first: > >>> > >>>+2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h > >>>+ Add AV_FRAME_FLAG_DISPOSABLE > >>>+ > >>>2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h > >>> 4K limit removed from avio_printf. > >>> > >>>diff --git a/libavutil/frame.h b/libavutil/frame.h > >>>index 5d3231e7bb..e1bf8795d2 100644 > >>>--- a/libavutil/frame.h > >>>+++ b/libavutil/frame.h > >>>@@ -522,6 +522,11 @@ typedef struct AVFrame { > >>>* A flag to mark the frames which need to be decoded, but shouldn't be output. > >>>*/ > >>>#define AV_FRAME_FLAG_DISCARD (1 << 2) > >>>+/** > >>>+ * A flag to indicate frames that can be discarded by the encoder. I.e. frames > >>>+ * that are an exact duplicate of the previous one. > >>>+ */ > >>>+#define AV_FRAME_FLAG_DISPOSABLE (1 << 3) > >> > >>Such a flag has the problem that it is not really an attribute of a frame in > >>itself, but of a frame of the frame chain. That can cause various issues: If > >>the first frame is dropped, the next frame no longer becomes disposable. > >> > > > >>For the issue we are trying to solve I feel that it is better if we create a > >>video filter which checks the buffers (and side data and crop parameters) if > >>they are the same and drops the frame. It also allows more granular control > >>(or setting parameters, like making sure you get a frame every 1 second or > >>so, which is common requirement even for VFR), plus it can work for existing > >>cases without this flag implemented. > > > >One disadvantage of this with the current ref counting system is that in codecs > >which do in place changes in a buffer. > >Keeping a reference to a frame means the decoder must copy the frame to get a > >new writable one to do changes in. Not keeping a ref avoids this. > >A method which detects duplicates without keeping a +1 reference on the > >previous frame would be better in this respect ... > > > > That is a fair point. I am OK with the frame flags then. a possible alternative would be some sort of unique id which is different for each decoder instance and only changes when any ref changes. This would avoid the problem of droping the first frame another one would be some sort of lightweight references which can be lost if no other reference is left. But this would probably requires more core changes iam fine with the flag, just dumping my thoughts here thx [...]
On 8/27/2019 10:03 AM, Michael Niedermayer wrote: > On Tue, Aug 27, 2019 at 10:58:13AM +0200, Marton Balint wrote: >> >> >> On Tue, 27 Aug 2019, Michael Niedermayer wrote: >> >>> On Tue, Aug 27, 2019 at 09:12:53AM +0200, Marton Balint wrote: >>>> >>>> >>>> On Mon, 26 Aug 2019, James Almer wrote: >>>> >>>>> Used to signal frames that can be safely discarded without losing >>>>> any picture data, side data, or metadata other than timing info. >>>>> >>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>> --- >>>>> This implements the "disposable frame" solution to allow library >>>>> users to drop duplicate frames before further processing if desired, >>>>> instead of forcing decoders to output vfr content when cfr is coded >>>>> in the bitstream. >>>>> >>>>> doc/APIchanges | 3 +++ >>>>> libavutil/frame.h | 5 +++++ >>>>> libavutil/version.h | 2 +- >>>>> 3 files changed, 9 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/doc/APIchanges b/doc/APIchanges >>>>> index 682b67aa25..b28d702bae 100644 >>>>> --- a/doc/APIchanges >>>>> +++ b/doc/APIchanges >>>>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 >>>>> >>>>> API changes, most recent first: >>>>> >>>>> +2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h >>>>> + Add AV_FRAME_FLAG_DISPOSABLE >>>>> + >>>>> 2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h >>>>> 4K limit removed from avio_printf. >>>>> >>>>> diff --git a/libavutil/frame.h b/libavutil/frame.h >>>>> index 5d3231e7bb..e1bf8795d2 100644 >>>>> --- a/libavutil/frame.h >>>>> +++ b/libavutil/frame.h >>>>> @@ -522,6 +522,11 @@ typedef struct AVFrame { >>>>> * A flag to mark the frames which need to be decoded, but shouldn't be output. >>>>> */ >>>>> #define AV_FRAME_FLAG_DISCARD (1 << 2) >>>>> +/** >>>>> + * A flag to indicate frames that can be discarded by the encoder. I.e. frames >>>>> + * that are an exact duplicate of the previous one. >>>>> + */ >>>>> +#define AV_FRAME_FLAG_DISPOSABLE (1 << 3) >>>> >>>> Such a flag has the problem that it is not really an attribute of a frame in >>>> itself, but of a frame of the frame chain. That can cause various issues: If >>>> the first frame is dropped, the next frame no longer becomes disposable. >>>> >>> >>>> For the issue we are trying to solve I feel that it is better if we create a >>>> video filter which checks the buffers (and side data and crop parameters) if >>>> they are the same and drops the frame. It also allows more granular control >>>> (or setting parameters, like making sure you get a frame every 1 second or >>>> so, which is common requirement even for VFR), plus it can work for existing >>>> cases without this flag implemented. >>> >>> One disadvantage of this with the current ref counting system is that in codecs >>> which do in place changes in a buffer. >>> Keeping a reference to a frame means the decoder must copy the frame to get a >>> new writable one to do changes in. Not keeping a ref avoids this. >>> A method which detects duplicates without keeping a +1 reference on the >>> previous frame would be better in this respect ... >>> >> >> That is a fair point. I am OK with the frame flags then. > > a possible alternative would be some sort of unique id which > is different for each decoder instance and only changes when > any ref changes. > This would avoid the problem of droping the first frame > another one would be some sort of lightweight references > which can be lost if no other reference is left. But this > would probably requires more core changes > > iam fine with the flag, just dumping my thoughts here > > thx I don't mind waiting a bit to further discuss how to signal these frames, but in the meantime i want to push patches 2 and 4 from this set, which are independent and fix ticket #7880 while reducing data copy as much as possible. I'll also send a patch to do the same as patch 4 from this set but for the nuv decoder.
diff --git a/doc/APIchanges b/doc/APIchanges index 682b67aa25..b28d702bae 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h + Add AV_FRAME_FLAG_DISPOSABLE + 2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h 4K limit removed from avio_printf. diff --git a/libavutil/frame.h b/libavutil/frame.h index 5d3231e7bb..e1bf8795d2 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -522,6 +522,11 @@ typedef struct AVFrame { * A flag to mark the frames which need to be decoded, but shouldn't be output. */ #define AV_FRAME_FLAG_DISCARD (1 << 2) +/** + * A flag to indicate frames that can be discarded by the encoder. I.e. frames + * that are an exact duplicate of the previous one. + */ +#define AV_FRAME_FLAG_DISPOSABLE (1 << 3) /** * @} */ diff --git a/libavutil/version.h b/libavutil/version.h index ecc6a7c9e2..658a508284 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 56 -#define LIBAVUTIL_VERSION_MINOR 33 +#define LIBAVUTIL_VERSION_MINOR 34 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
Used to signal frames that can be safely discarded without losing any picture data, side data, or metadata other than timing info. Signed-off-by: James Almer <jamrial@gmail.com> --- This implements the "disposable frame" solution to allow library users to drop duplicate frames before further processing if desired, instead of forcing decoders to output vfr content when cfr is coded in the bitstream. doc/APIchanges | 3 +++ libavutil/frame.h | 5 +++++ libavutil/version.h | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-)