| Message ID | 20171201002209.24270-2-jstebbins@jetheaddev.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, 30 Nov 2017, John Stebbins wrote: > Used by movenc to fill sdtp box > --- > libavcodec/libx265.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c > index 4456e300f2..c137fe4ae1 100644 > --- a/libavcodec/libx265.c > +++ b/libavcodec/libx265.c > @@ -329,6 +329,9 @@ FF_DISABLE_DEPRECATION_WARNINGS > FF_ENABLE_DEPRECATION_WARNINGS > #endif > > + if (x265pic_out.sliceType == X265_TYPE_B) > + pkt->flags |= AV_PKT_FLAG_DISPOSABLE; You can only set this for B pictures which are not referenced by other frames, no? Otherwise they can't be dropped independently from the others. Regards, Marton
On 12/02/2017 01:40 AM, Marton Balint wrote: > On Thu, 30 Nov 2017, John Stebbins wrote: > >> Used by movenc to fill sdtp box >> --- >> libavcodec/libx265.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c >> index 4456e300f2..c137fe4ae1 100644 >> --- a/libavcodec/libx265.c >> +++ b/libavcodec/libx265.c >> @@ -329,6 +329,9 @@ FF_DISABLE_DEPRECATION_WARNINGS >> FF_ENABLE_DEPRECATION_WARNINGS >> #endif >> >> + if (x265pic_out.sliceType == X265_TYPE_B) >> + pkt->flags |= AV_PKT_FLAG_DISPOSABLE; > You can only set this for B pictures which are not referenced by other > frames, no? Otherwise they can't be dropped independently from the others. > > Correct. x265 version 2.5 and above returns X265_TYPE_BREF for frames that are referenced. There is a way to distinguish on version 2.4 and below as well using x265pic_out.frameData.sliceType, and an earlier version of my patch used that. But it's really not the "right" way going forward. I can restore it if supporting earlier versions of x265 is important. But my feeling with such fast moving projects as x265 is it's best not to keep too much cruft in support of old versions.
On 12/2/2017 1:52 PM, John Stebbins wrote: > On 12/02/2017 01:40 AM, Marton Balint wrote: >> On Thu, 30 Nov 2017, John Stebbins wrote: >> >>> Used by movenc to fill sdtp box >>> --- >>> libavcodec/libx265.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c >>> index 4456e300f2..c137fe4ae1 100644 >>> --- a/libavcodec/libx265.c >>> +++ b/libavcodec/libx265.c >>> @@ -329,6 +329,9 @@ FF_DISABLE_DEPRECATION_WARNINGS >>> FF_ENABLE_DEPRECATION_WARNINGS >>> #endif >>> >>> + if (x265pic_out.sliceType == X265_TYPE_B) >>> + pkt->flags |= AV_PKT_FLAG_DISPOSABLE; >> You can only set this for B pictures which are not referenced by other >> frames, no? Otherwise they can't be dropped independently from the others. >> >> > > Correct. x265 version 2.5 and above returns X265_TYPE_BREF for frames that are referenced. There is a way to > distinguish on version 2.4 and below as well using x265pic_out.frameData.sliceType, and an earlier version of my patch > used that. But it's really not the "right" way going forward. I can restore it if supporting earlier versions of x265 > is important. But my feeling with such fast moving projects as x265 is it's best not to keep too much cruft in support > of old versions. You can bump the minimum required version. Just make the necessary changes in configure.
On 12/02/2017 09:25 AM, James Almer wrote: > On 12/2/2017 1:52 PM, John Stebbins wrote: >> On 12/02/2017 01:40 AM, Marton Balint wrote: >>> On Thu, 30 Nov 2017, John Stebbins wrote: >>> >>>> Used by movenc to fill sdtp box >>>> --- >>>> libavcodec/libx265.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c >>>> index 4456e300f2..c137fe4ae1 100644 >>>> --- a/libavcodec/libx265.c >>>> +++ b/libavcodec/libx265.c >>>> @@ -329,6 +329,9 @@ FF_DISABLE_DEPRECATION_WARNINGS >>>> FF_ENABLE_DEPRECATION_WARNINGS >>>> #endif >>>> >>>> + if (x265pic_out.sliceType == X265_TYPE_B) >>>> + pkt->flags |= AV_PKT_FLAG_DISPOSABLE; >>> You can only set this for B pictures which are not referenced by other >>> frames, no? Otherwise they can't be dropped independently from the others. >>> >>> >> Correct. x265 version 2.5 and above returns X265_TYPE_BREF for frames that are referenced. There is a way to >> distinguish on version 2.4 and below as well using x265pic_out.frameData.sliceType, and an earlier version of my patch >> used that. But it's really not the "right" way going forward. I can restore it if supporting earlier versions of x265 >> is important. But my feeling with such fast moving projects as x265 is it's best not to keep too much cruft in support >> of old versions. > You can bump the minimum required version. Just make the necessary > changes in configure. > Ah, good point. That should be done, or I should add back support for earlier versions. Is there any desire by anyone to keep support for earlier versions?
2017-12-02 18:37 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>: > That should be done, or I should add back support for earlier versions. > Is there any desire by anyone to keep support for earlier versions? How old is 2.5, is 2.4 used by current versions of distributions? (How ugly is the support for earlier versions?) Carl Eugen
On 12/2/2017 2:37 PM, John Stebbins wrote: > On 12/02/2017 09:25 AM, James Almer wrote: >> On 12/2/2017 1:52 PM, John Stebbins wrote: >>> On 12/02/2017 01:40 AM, Marton Balint wrote: >>>> On Thu, 30 Nov 2017, John Stebbins wrote: >>>> >>>>> Used by movenc to fill sdtp box >>>>> --- >>>>> libavcodec/libx265.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c >>>>> index 4456e300f2..c137fe4ae1 100644 >>>>> --- a/libavcodec/libx265.c >>>>> +++ b/libavcodec/libx265.c >>>>> @@ -329,6 +329,9 @@ FF_DISABLE_DEPRECATION_WARNINGS >>>>> FF_ENABLE_DEPRECATION_WARNINGS >>>>> #endif >>>>> >>>>> + if (x265pic_out.sliceType == X265_TYPE_B) >>>>> + pkt->flags |= AV_PKT_FLAG_DISPOSABLE; >>>> You can only set this for B pictures which are not referenced by other >>>> frames, no? Otherwise they can't be dropped independently from the others. >>>> >>>> >>> Correct. x265 version 2.5 and above returns X265_TYPE_BREF for frames that are referenced. There is a way to >>> distinguish on version 2.4 and below as well using x265pic_out.frameData.sliceType, and an earlier version of my patch >>> used that. But it's really not the "right" way going forward. I can restore it if supporting earlier versions of x265 >>> is important. But my feeling with such fast moving projects as x265 is it's best not to keep too much cruft in support >>> of old versions. >> You can bump the minimum required version. Just make the necessary >> changes in configure. >> > > Ah, good point. That should be done, or I should add back support for earlier versions. Is there any desire by anyone > to keep support for earlier versions? No, 2.5 is not even the most recent version, so it's fine as the minimum supported version.
On 12/2/2017 2:40 PM, Carl Eugen Hoyos wrote: > 2017-12-02 18:37 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>: >> That should be done, or I should add back support for earlier versions. >> Is there any desire by anyone to keep support for earlier versions? > > How old is 2.5, is 2.4 used by current versions of distributions? > (How ugly is the support for earlier versions?) It's four months old, and rolling release distros are using it and will move on to 2.6 soon. By the time non rolling release distros switch to ffmpeg > 3.4, they will also switch to whatever is newest for x265 at the time.
2017-12-02 18:51 GMT+01:00 James Almer <jamrial@gmail.com>: > On 12/2/2017 2:40 PM, Carl Eugen Hoyos wrote: >> 2017-12-02 18:37 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>: >>> That should be done, or I should add back support for earlier versions. >>> Is there any desire by anyone to keep support for earlier versions? >> >> How old is 2.5, is 2.4 used by current versions of distributions? >> (How ugly is the support for earlier versions?) > > It's four months old, and rolling release distros are using it and will > move on to 2.6 soon. > > By the time non rolling release distros switch to ffmpeg > 3.4, they > will also switch to whatever is newest for x265 at the time. I was mostly thinking about users who build FFmpeg by themselves on current distributions. (And about developers who like to test on vanilla systems.) Four months seem not a lot to me. (I believe it is impossible that your criteria ever fails.) Carl Eugen
On 12/2/2017 3:00 PM, Carl Eugen Hoyos wrote: > 2017-12-02 18:51 GMT+01:00 James Almer <jamrial@gmail.com>: >> On 12/2/2017 2:40 PM, Carl Eugen Hoyos wrote: >>> 2017-12-02 18:37 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>: >>>> That should be done, or I should add back support for earlier versions. >>>> Is there any desire by anyone to keep support for earlier versions? >>> >>> How old is 2.5, is 2.4 used by current versions of distributions? >>> (How ugly is the support for earlier versions?) >> >> It's four months old, and rolling release distros are using it and will >> move on to 2.6 soon. >> >> By the time non rolling release distros switch to ffmpeg > 3.4, they >> will also switch to whatever is newest for x265 at the time. > > I was mostly thinking about users who build FFmpeg by themselves > on current distributions. (And about developers who like to test on > vanilla systems.) Those on rolling release ditros are covered, and those compiling ffmpeg git head on non rolling release distros are most likely also compiling any required libraries for it. Hell, 2.5 is even in Debian testing right now. I very much rather avoid ifdeffery to support old releases from projects with a rapid update schedule like x265.
On Sat, Dec 02, 2017 at 03:26:34PM -0300, James Almer wrote: > On 12/2/2017 3:00 PM, Carl Eugen Hoyos wrote: > > 2017-12-02 18:51 GMT+01:00 James Almer <jamrial@gmail.com>: > >> On 12/2/2017 2:40 PM, Carl Eugen Hoyos wrote: > >>> 2017-12-02 18:37 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>: > >>>> That should be done, or I should add back support for earlier versions. > >>>> Is there any desire by anyone to keep support for earlier versions? > >>> > >>> How old is 2.5, is 2.4 used by current versions of distributions? > >>> (How ugly is the support for earlier versions?) > >> > >> It's four months old, and rolling release distros are using it and will > >> move on to 2.6 soon. > >> > >> By the time non rolling release distros switch to ffmpeg > 3.4, they > >> will also switch to whatever is newest for x265 at the time. > > > > I was mostly thinking about users who build FFmpeg by themselves > > on current distributions. (And about developers who like to test on > > vanilla systems.) > > Those on rolling release ditros are covered, and those compiling ffmpeg > git head on non rolling release distros are most likely also compiling > any required libraries for it. > Hell, 2.5 is even in Debian testing right now. > > I very much rather avoid ifdeffery to support old releases from projects > with a rapid update schedule like x265. I understand why you prefer this but i think its nicer to our users also the stricter version deps are the more one runs into issues for example the "recent" libvpx min version bump required me to update my libvpx and it promptly broke many older FFmpeg versions ive patched my release branches locally and that would be in the next point releases of course but versions released previously require a libvpx that master doesnt build with and what master builds with they dont. If we accumulate too many of these things bisect will become increasingly painfull short version, my "vote" is for keeping support for the older version by #if or any other solution [...]
On 12/2/2017 10:48 PM, Michael Niedermayer wrote: > On Sat, Dec 02, 2017 at 03:26:34PM -0300, James Almer wrote: >> On 12/2/2017 3:00 PM, Carl Eugen Hoyos wrote: >>> 2017-12-02 18:51 GMT+01:00 James Almer <jamrial@gmail.com>: >>>> On 12/2/2017 2:40 PM, Carl Eugen Hoyos wrote: >>>>> 2017-12-02 18:37 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>: >>>>>> That should be done, or I should add back support for earlier versions. >>>>>> Is there any desire by anyone to keep support for earlier versions? >>>>> >>>>> How old is 2.5, is 2.4 used by current versions of distributions? >>>>> (How ugly is the support for earlier versions?) >>>> >>>> It's four months old, and rolling release distros are using it and will >>>> move on to 2.6 soon. >>>> >>>> By the time non rolling release distros switch to ffmpeg > 3.4, they >>>> will also switch to whatever is newest for x265 at the time. >>> >>> I was mostly thinking about users who build FFmpeg by themselves >>> on current distributions. (And about developers who like to test on >>> vanilla systems.) >> >> Those on rolling release ditros are covered, and those compiling ffmpeg >> git head on non rolling release distros are most likely also compiling >> any required libraries for it. >> Hell, 2.5 is even in Debian testing right now. >> > >> I very much rather avoid ifdeffery to support old releases from projects >> with a rapid update schedule like x265. > > I understand why you prefer this but i think its nicer to our users > also the stricter version deps are the more one runs into issues > > for example the "recent" libvpx min version bump required me to > update my libvpx How? 1.4.0 is two years and a half old. Even Debian ships 1.6.x in stable by now. > and it promptly broke many older FFmpeg versions > ive patched my release branches locally and that would be in the next > point releases of course but versions released previously require > a libvpx that master doesnt build with and what master builds with > they dont. > > If we accumulate too many of these things bisect will become > increasingly painfull We can't keep external libraries hostage of bisect attempts for unrelated modules... You don't need libvpx or libx265 compiled in when hunting down a bug in mpeg2/h264/snow code. We'd never be able to update anything that way. I'm surprised for that matter that the libvpx wrappers in old FFMpeg versions don't work with >= 1.4.0. I don't recall any kind of API break that we had to work around in them. > > short version, my "vote" is for keeping support for the older version > by #if or any other solution I'm fine keeping libx265 as is for now. Unlike libvpx 1.4.0, which is two years and a half old, libx265 2.5 is admittedly somewhat recent. But bisecting bugs in unrelated modules is definitely not a reason to maintain ugly support for old and potentially buggy/unstable/insecure versions of optional, non system external libraries.
On 12/02/2017 06:32 PM, James Almer wrote: > On 12/2/2017 10:48 PM, Michael Niedermayer wrote: >> On Sat, Dec 02, 2017 at 03:26:34PM -0300, James Almer wrote: >>> On 12/2/2017 3:00 PM, Carl Eugen Hoyos wrote: >>>> 2017-12-02 18:51 GMT+01:00 James Almer <jamrial@gmail.com>: >>>>> On 12/2/2017 2:40 PM, Carl Eugen Hoyos wrote: >>>>>> 2017-12-02 18:37 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>: >>>>>>> That should be done, or I should add back support for earlier versions. >>>>>>> Is there any desire by anyone to keep support for earlier versions? >>>>>> How old is 2.5, is 2.4 used by current versions of distributions? >>>>>> (How ugly is the support for earlier versions?) >>>>> It's four months old, and rolling release distros are using it and will >>>>> move on to 2.6 soon. >>>>> >>>>> By the time non rolling release distros switch to ffmpeg > 3.4, they >>>>> will also switch to whatever is newest for x265 at the time. >>>> I was mostly thinking about users who build FFmpeg by themselves >>>> on current distributions. (And about developers who like to test on >>>> vanilla systems.) >>> Those on rolling release ditros are covered, and those compiling ffmpeg >>> git head on non rolling release distros are most likely also compiling >>> any required libraries for it. >>> Hell, 2.5 is even in Debian testing right now. >>> >>> I very much rather avoid ifdeffery to support old releases from projects >>> with a rapid update schedule like x265. >> I understand why you prefer this but i think its nicer to our users >> also the stricter version deps are the more one runs into issues >> >> for example the "recent" libvpx min version bump required me to >> update my libvpx > How? 1.4.0 is two years and a half old. Even Debian ships 1.6.x in > stable by now. > >> and it promptly broke many older FFmpeg versions >> ive patched my release branches locally and that would be in the next >> point releases of course but versions released previously require >> a libvpx that master doesnt build with and what master builds with >> they dont. >> >> If we accumulate too many of these things bisect will become >> increasingly painfull > We can't keep external libraries hostage of bisect attempts for > unrelated modules... You don't need libvpx or libx265 compiled in when > hunting down a bug in mpeg2/h264/snow code. We'd never be able to update > anything that way. > > I'm surprised for that matter that the libvpx wrappers in old FFMpeg > versions don't work with >= 1.4.0. I don't recall any kind of API break > that we had to work around in them. > >> short version, my "vote" is for keeping support for the older version >> by #if or any other solution > I'm fine keeping libx265 as is for now. Unlike libvpx 1.4.0, which is > two years and a half old, libx265 2.5 is admittedly somewhat recent. > But bisecting bugs in unrelated modules is definitely not a reason to > maintain ugly support for old and potentially buggy/unstable/insecure > versions of optional, non system external libraries. > I'll add back the support for libx265 <= 2.4 then. There's technically no ifdef'ery required. But it requires using a field in a struct (x265_frame_stats) that is really meant for logging. My concern with using it is that logging could change in the future and break this. So I'll ifdef it anyway so that it's more future proof.
On Sat, Dec 02, 2017 at 11:32:16PM -0300, James Almer wrote: > On 12/2/2017 10:48 PM, Michael Niedermayer wrote: > > On Sat, Dec 02, 2017 at 03:26:34PM -0300, James Almer wrote: [...] > I'm surprised for that matter that the libvpx wrappers in old FFMpeg > versions don't work with >= 1.4.0. I don't recall any kind of API break > that we had to work around in them. i dont know about exact versions, i just know some release branches stoped building with configure settings that worked previously after i updated libvpx i needed to backport this: commit 6540fe04a3f9a11ba7084a49b3ee5fa2fc5b32ab Author: James Zern <jzern@google.com> Date: Mon Oct 19 22:44:11 2015 -0700 libvpxenc: remove some unused ctrl id mappings VP8E_UPD_ENTROPY, VP8E_UPD_REFERENCE, VP8E_USE_REFERENCE were removed from libvpx and the remaining values were never used here Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: James Zern <jzern@google.com> [...]
On Sun, Dec 3, 2017 at 10:42 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sat, Dec 02, 2017 at 11:32:16PM -0300, James Almer wrote: >> On 12/2/2017 10:48 PM, Michael Niedermayer wrote: >> > On Sat, Dec 02, 2017 at 03:26:34PM -0300, James Almer wrote: > [...] > >> I'm surprised for that matter that the libvpx wrappers in old FFMpeg >> versions don't work with >= 1.4.0. I don't recall any kind of API break >> that we had to work around in them. > > i dont know about exact versions, i just know some release branches > stoped building with configure settings that worked previously after i > updated libvpx > > i needed to backport this: > That change is kind of the opposite of what we're really talking about here, isn't it. It provides support for newer versions, didn't remove support for older ones. - Hendrik
On Sun, Dec 03, 2017 at 11:05:36PM +0100, Hendrik Leppkes wrote: > On Sun, Dec 3, 2017 at 10:42 PM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > On Sat, Dec 02, 2017 at 11:32:16PM -0300, James Almer wrote: > >> On 12/2/2017 10:48 PM, Michael Niedermayer wrote: > >> > On Sat, Dec 02, 2017 at 03:26:34PM -0300, James Almer wrote: > > [...] > > > >> I'm surprised for that matter that the libvpx wrappers in old FFMpeg > >> versions don't work with >= 1.4.0. I don't recall any kind of API break > >> that we had to work around in them. > > > > i dont know about exact versions, i just know some release branches > > stoped building with configure settings that worked previously after i > > updated libvpx > > > > i needed to backport this: > > > > That change is kind of the opposite of what we're really talking about > here, isn't it. It provides support for newer versions, didn't remove > support for older ones. yes, there was a 2nd change that droped support for older versions one to add support for newer then ater one to drop old so new doesnt work before both and old doesnt work after both backporting the first fixes this for refernce: for the other change "git log" points me to this, so i assume it was what removed support for old versions commit b765a04550ff6f954a57bf28158511d260f2e087 Author: James Zern <jzern@google.com> Date: Fri Nov 17 17:24:07 2017 -0800 configure: require libvpx-1.4.0 for vp[89] support this will simplify libvpxenc/dec.c and ensure more stable versions of the codecs are present. Reviewed-by: James Almer <jamrial@gmail.com> Signed-off-by: James Zern <jzern@google.com> [...]
diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c index 4456e300f2..c137fe4ae1 100644 --- a/libavcodec/libx265.c +++ b/libavcodec/libx265.c @@ -329,6 +329,9 @@ FF_DISABLE_DEPRECATION_WARNINGS FF_ENABLE_DEPRECATION_WARNINGS #endif + if (x265pic_out.sliceType == X265_TYPE_B) + pkt->flags |= AV_PKT_FLAG_DISPOSABLE; + *got_packet = 1; return 0; }