Re: [FFmpeg-devel] [PATCH] libavformat/mpegts.c: ignore a section with next flag
sorry, I don't have permission to push the commits.
I've misunderstood the Developer Document.
On 2022/04/07 18:26, TADANO Tokumei wrote:
will apply
On 2022/04/03 19:23, TADANO Tokumei wrote:
'current_next_indicator' of 0 (next) on each section header indicates
the service information is for immediate future one.
ffmpeg doesn't need to parse it but current (1) one.
ref: section 5.1.1 of DVB BlueBook A038 (EN 300 468)
Signed-off-by: TADANO Tokumei
---
libavformat/mpegts.c | 8
1 file changed, 8 insertions(+)
diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index ecffb01562..49f7735123 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -645,6 +645,7 @@ typedef struct SectionHeader {
uint8_t tid;
uint16_t id;
uint8_t version;
+ uint8_t current_next;
uint8_t sec_num;
uint8_t last_sec_num;
} SectionHeader;
@@ -773,6 +774,7 @@ static int parse_section_header(SectionHeader *h,
if (val < 0)
return val;
h->version = (val >> 1) & 0x1f;
+ h->current_next = val & 0x01;
val = get8(pp, p_end);
if (val < 0)
return val;
@@ -2332,6 +2334,8 @@ static void pmt_cb(MpegTSFilter *filter, const uint8_t
*section, int section_len
return;
if (h->tid != PMT_TID)
return;
+ if (!h->current_next)
+ return;
if (skip_identical(h, tssf))
return;
@@ -2541,6 +2545,8 @@ static void pat_cb(MpegTSFilter *filter, const uint8_t
*section, int section_len
return;
if (h->tid != PAT_TID)
return;
+ if (!h->current_next)
+ return;
if (ts->skip_changes)
return;
@@ -2679,6 +2685,8 @@ static void sdt_cb(MpegTSFilter *filter, const uint8_t
*section, int section_len
return;
if (h->tid != SDT_TID)
return;
+ if (!h->current_next)
+ return;
if (ts->skip_changes)
return;
if (skip_identical(h, tssf))
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] movenc: add write_btrt option
On Fri, Apr 8, 2022 at 5:47 AM "zhilizhao(赵志立)" wrote:
>
>
>
> > On Apr 8, 2022, at 4:36 AM, Jan Ekström wrote:
> >
> > On Thu, Apr 7, 2022 at 11:42 AM Eran Kornblau
> > wrote:
> >>
> >>>
> >>>
> >>> -Original Message-
> >>> From: ffmpeg-devel On Behalf Of
> >>> "zhilizhao(???)"
> >>> Sent: Wednesday, 6 April 2022 11:46
> >>> To: FFmpeg development discussions and patches
> >>> Subject: Re: [FFmpeg-devel] movenc: add write_btrt option
> >>>
> On Apr 3, 2022, at 1:07 PM, Eran Kornblau
> wrote:
>
> Trying my luck in a new thread…
>
> This patch is in continuation to this discussion –
> https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fffmp
> eg.org%2Fpipermail%2Fffmpeg-devel%2F2022-March%2F294623.html&data=
> 04%7C01%7C%7Cb9907958f97048f5645708da17a9f3c8%7C0c503748de3f4e2597e268
> 19d53a42b6%7C1%7C0%7C637848315958196733%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am
> p;sdata=flQc21b5EVWTy%2Bkmw%2FncIWzdLqUxY5XFislPRs5Ij6o%3D&reserve
> d=0
>
> supports forcing or disabling the writing of the btrt atom.
> the default behavior is to write the atom only for mp4 mode.
> ---
> libavformat/movenc.c | 30 +++---
> libavformat/movenc.h | 1 +
> 2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c index
> 4c868919ae..b75f1c6909 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
>
> >>> […]
>
> -if (track->mode == MODE_MP4 &&
> -((ret = mov_write_btrt_tag(pb, track)) < 0))
> -return ret;
> +if ((mov->write_btrt == -1 && track->mode == MODE_MP4) ||
> mov->write_btrt == 1) {
> +if ((ret = mov_write_btrt_tag(pb, track)) < 0) {
> +return ret;
> +}
> +}
> >>>
> >>> I prefer to handle the auto mode (mov->write_btrt == -1) in a single
> >>> place, so we don’t need to change multiple lines if the condition
> >>> changed, e.g., enable btrt for MODE_MOV. Please correct me if I’m wrong,
> >>> mov_init() has all of the contexts to overwrite mov->write_btrt.
> >>>
> >> Makes sense, thanks for the feedback!
> >> Updated patch attached
> >>
> >> Eran
> >
> > Generally speaking I am not against this patch. Would have possibly
> > came up with something similar myself in case the actual output of
> > libavformat would have caused issues, which surprisingly enough it
> > wasn't.
> >
> > I know you have just copied the logic from tmcd or so, but I think the
> > -1 logic is unnecessary. We don't need to force writing of this
> > structure no matter what, so you either have it enabled (by default),
> > or disabled. If additional formats such as QTFF have since added the
> > btrt box into their specification, that doesn't need forcing, but
> > rather addition of it into the logic later (if you wanted forcing then
> > you'd have to deal with strict_std_compliance being
> > unofficial/experimental or higher etc, and if this was not set -
> > warning the user that a not officially defined functionality was being
> > written into the container and exiting with AVERROR_EXPERIMENTAL).
> >
> > Additionally, I thought new options go to the end of the AVOption
> > array, but then saw 1dddb930aaf0cadaa19f86e81225c9c352745262 where
> > James added "crf" into the middle of an array... so I guess since it's
> > an array and not a struct the location no longer matters as much?
> > ┐(´д｀)┌ Although the struct integer should definitely go to the end of
> > it, otherwise you are breaking existing offsets? Although thankfully,
> > the struct isn't externally exposed so someone else could chime in
> > regarding this, I am unfortunately quite tired throughout this week :P
> > .
>
> The order of options and the offset of fields in private struct have no
> effect on ABI. I take these into consideration:
> 1. Readability. Related options and fields should be put at the same
>place.
> 2. Memory footprint. Reduce struct padding.
>
Yes, this is a minor thing within my comment, my comment was mostly
regarding the -1 case being unnecessary (since I don't think we need
to actually force-force this, just controlling whether this box is
written or not under the general rules of where it is defined).
And yes, if the order doesn't matter then grouping makes sense. It's
just that for very long "add to the end" was the general principle, so
I was mostly utilizing this as a base to request comments from others
regarding this.
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of > Anton Khirnov > Sent: Thursday, April 7, 2022 10:33 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > Quoting Soft Works (2022-04-06 18:29:53) > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > > > Anton Khirnov > > > Sent: Wednesday, April 6, 2022 10:42 AM > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > > > architecture > > > > > > Quoting Soft Works (2022-04-05 23:05:57) > > > > do I understand it right that there won't be a single-thread > > > > operation mode that replicates/corresponds the current behavior? > > > > > > Correct, maintaining a single-threaded mode would require massive > > > amounts of extra effort for questionable gain. > > > > The gain is not to be seen in having an alternate run-mode in > > longer-term term perspective. It is about avoiding a single > > point-of-no-return change which may have fundamental consequences > > and impose debt on other developers when it would no longer be > possible > > to compare to the previous mode of operation. > > I don't understand where your apocalyptic language is coming from. > Yes, > overall this will be a very big change. Should it turn out that the switch will go smoothly and easily without (minimal) issues, then I'll sincerely admit that my concerns were exaggerated and unjustified. > > Threads are not magic. It is ubiquitous well-understood technology > that > has been around for decades. Yes, they do involve their own > challenges, > but so does everything else, including the status quo. > > The current architecture of ffmpeg.c is, in my opinion, unsustainable. Agreed. > There is barely any separation of responsibilities. E.g. filtering > code > will modify muxer state, etc. It is very hard to understand the code > behaviour in various scenarious (we have SO MANY options), and how > will > some code change affect it. This makes adding new major features much > more painful than it should be. I am firmly convinced that these > patches > will make the code significantly easier to understand. No doubt about that. > Sorry, this does not seem a reasonable request to me. For one thing, > the > only advantage you claim involve highly hypothetical doomsday > scenarios > where you cannot compare to the old code and so ALL IS LOST. That > smells > like a cargo cult. The current code is not holy scripture determining > how things should work for all eternity. It has bugs, inconsistencies, > and poorly handled corner cases (some of which are fixed by this very > patchset). As mentioned, the concern is about the transition not about carrying this for a long time. > Furthermore, remember that this is just the first step. There will be > further patchsets converting the other components. I intend to > upstream > them gradually one after the other. Your suggestion would require me > to > instead write the whole thing at once, fighting rebase conflicts all > the > way, and then submit it as a giant utterly unreviewable patchset. That's not what I meant, but anyway it's not worth discussing when it's a minority opinion. Just a practical question instead for planning purposes: Which timeframe do you expect for the whole process? When do you plan to start and for how long do you think it will take until all further patchsets will be submitted/applied? Thanks, sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_colorkey: add >8 depth support
Looks sensible to me. Didn't have time to test it, but assuming you verified nothing broke and the new stuff works fine, you have my approval. Will give it a test soon as well. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_colorkey: add >8 depth support
On Fri, Apr 8, 2022 at 5:52 PM Timo Rothenpieler wrote: > Looks sensible to me. Didn't have time to test it, but assuming you > verified nothing broke and the new stuff works fine, you have my approval. > > I made some extra changes to make filter at same speed or even faster than before. > Will give it a test soon as well. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/alimiter: Add "flush_buffer" option to flush the remaining valid data to the output
On Thu, Apr 7, 2022 at 11:56 PM Wang Cao wrote: > On Thu, Apr 7, 2022 at 12:44 AM Paul B Mahol wrote: > > > On Wed, Apr 6, 2022 at 1:49 PM Paul B Mahol wrote: > > > > > > > > > > > On Tue, Apr 5, 2022 at 8:57 PM Wang Cao < > > wangcao-at-google@ffmpeg.org> > > > wrote: > > > > > >> On Mon, Apr 4, 2022 at 3:28 PM Marton Balint wrote: > > >> > > >> > > > >> > > > >> > On Mon, 4 Apr 2022, Paul B Mahol wrote: > > >> > > > >> > > On Sun, Mar 27, 2022 at 11:41 PM Marton Balint > > wrote: > > >> > > > > >> > >> > > >> > >> > > >> > >> On Sat, 26 Mar 2022, Wang Cao wrote: > > >> > >> > > >> > >>> The change in the commit will add some samples to the end of the > > >> audio > > >> > >>> stream. The intention is to add a "zero_delay" option eventually > > to > > >> not > > >> > >>> have the delay in the begining the output from alimiter due to > > >> > >>> lookahead. > > >> > >> > > >> > >> I was very much suprised to see that the alimiter filter actually > > >> delays > > >> > >> the audio - as in extra samples are inserted in the beginning and > > >> some > > >> > >> samples are cut in the end. This trashes A-V sync, so it is a bug > > >> IMHO. > > >> > >> > > >> > >> So unless somebody has some valid usecase for the legacy way of > > >> > operation > > >> > >> I'd just simply change it to be "zero delay" without any > additional > > >> user > > >> > >> option, in a single patch. > > >> > >> > > >> > > > > >> > > > > >> > > This is done by this patch in very complicated way and also it > > really > > >> > > should be optional. > > >> > > > >> > But why does it make sense to keep the current (IMHO buggy) > > operational > > >> > mode which adds silence in the beginning and trims the end? I > > understand > > >> > that the original implementation worked like this, but libavfilter > has > > >> > packet timestamps and N:M filtering so there is absolutely no reason > > to > > >> > use an 1:1 implementation and live with its limitations. > > >> > > > >> Hello Paul and Marton, thank you so much for taking time to review my > > >> patch. > > >> I totally understand that my patch may seem a little bit complicated > > but I > > >> can > > >> show with a FATE test that if we set the alimiter to behave as a > > >> passthrough filter, > > >> the output frames will be the same from "framecrc" with my patch. The > > >> existing > > >> behavior will not work for all gapless audio processing. > > >> > > >> The complete patch to fix this issue is at > > >> > > >> > > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220330210314.2055201-1-wang...@google.com/ > > >> > > >> Regarding Paul's concern, I personally don't have any preference > whether > > >> to > > >> put > > >> the patch as an extra option or not. With respect to the > implementation, > > >> the patch > > >> is the best I can think of by preserving as much information as > possible > > >> from input > > >> frames. I also understand it may break concept that "filter_frame" > > outputs > > >> one frame > > >> at a time. For alimiter with my patch, depending on the size of the > > >> lookahead buffer, > > >> it may take a few frames before one output frame can be generated. > This > > is > > >> inevitable > > >> to compensate for the delay of the lookahead buffer. > > >> > > >> Thanks again for reviewing my patch and I'm looking forward to hearing > > >> from > > >> you :) > > >> > > > > > > Better than (because its no more 1 frame X nb_samples in, 1 frame X > > > nb_samples out) to replace .filter_frame/.request_frame with .activate > > > logic. > > > > > > And make this output delay compensation filtering optional. > > > > > > In this process make sure that output PTS frame timestamps are > unchanged > > > from input one, by keeping reference of needed frames in filter queue. > > > > > > Look how speechnorm/dynaudnorm does it. > > > > > > > > > Alternatively, use current logic in ladspa filter, (also add option with > > same name). > > > > This would need less code, and probably better approach, as there is no > > extra latency introduced. > > > > Than mapping 1:1 between same number of samples per frame is not hold any > > more, but I think that is not much important any more. > > > Thank you for replying to me with your valuable feedback! I have checked > af_ladspa > and the "request_frame" function in af_ladspa looks similar to what I'm > doing. The > difference comes from the fact that I need an internal frame buffer to keep > track of > output frames. Essentially I add a frame to the internal buffer when an > input frame > comes in. The frames in this buffer will be the future output frames. We > start writing > these output frame data buffers only when the internal lookahead buffer has > been filled. > When there is no more input frames, we flushing all the remaining data in > the internal > frame buffers and lookahead buffers. Can you advise on my approach here? > Thank you! > > I can put my current implementation of "filter_frame" and "request_frame
Re: [FFmpeg-devel] [PATCH] avfilter/alimiter: Add "flush_buffer" option to flush the remaining valid data to the output
On Fri, Apr 8, 2022 at 11:40 AM Paul B Mahol wrote: > On Thu, Apr 7, 2022 at 11:56 PM Wang Cao > > wrote: > > > On Thu, Apr 7, 2022 at 12:44 AM Paul B Mahol wrote: > > > > > On Wed, Apr 6, 2022 at 1:49 PM Paul B Mahol wrote: > > > > > > > > > > > > > > > On Tue, Apr 5, 2022 at 8:57 PM Wang Cao < > > > wangcao-at-google@ffmpeg.org> > > > > wrote: > > > > > > > >> On Mon, Apr 4, 2022 at 3:28 PM Marton Balint wrote: > > > >> > > > >> > > > > >> > > > > >> > On Mon, 4 Apr 2022, Paul B Mahol wrote: > > > >> > > > > >> > > On Sun, Mar 27, 2022 at 11:41 PM Marton Balint > > > wrote: > > > >> > > > > > >> > >> > > > >> > >> > > > >> > >> On Sat, 26 Mar 2022, Wang Cao wrote: > > > >> > >> > > > >> > >>> The change in the commit will add some samples to the end of > the > > > >> audio > > > >> > >>> stream. The intention is to add a "zero_delay" option > eventually > > > to > > > >> not > > > >> > >>> have the delay in the begining the output from alimiter due to > > > >> > >>> lookahead. > > > >> > >> > > > >> > >> I was very much suprised to see that the alimiter filter > actually > > > >> delays > > > >> > >> the audio - as in extra samples are inserted in the beginning > and > > > >> some > > > >> > >> samples are cut in the end. This trashes A-V sync, so it is a > bug > > > >> IMHO. > > > >> > >> > > > >> > >> So unless somebody has some valid usecase for the legacy way of > > > >> > operation > > > >> > >> I'd just simply change it to be "zero delay" without any > > additional > > > >> user > > > >> > >> option, in a single patch. > > > >> > >> > > > >> > > > > > >> > > > > > >> > > This is done by this patch in very complicated way and also it > > > really > > > >> > > should be optional. > > > >> > > > > >> > But why does it make sense to keep the current (IMHO buggy) > > > operational > > > >> > mode which adds silence in the beginning and trims the end? I > > > understand > > > >> > that the original implementation worked like this, but libavfilter > > has > > > >> > packet timestamps and N:M filtering so there is absolutely no > reason > > > to > > > >> > use an 1:1 implementation and live with its limitations. > > > >> > > > > >> Hello Paul and Marton, thank you so much for taking time to review > my > > > >> patch. > > > >> I totally understand that my patch may seem a little bit complicated > > > but I > > > >> can > > > >> show with a FATE test that if we set the alimiter to behave as a > > > >> passthrough filter, > > > >> the output frames will be the same from "framecrc" with my patch. > The > > > >> existing > > > >> behavior will not work for all gapless audio processing. > > > >> > > > >> The complete patch to fix this issue is at > > > >> > > > >> > > > > > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220330210314.2055201-1-wang...@google.com/ > > > >> > > > >> Regarding Paul's concern, I personally don't have any preference > > whether > > > >> to > > > >> put > > > >> the patch as an extra option or not. With respect to the > > implementation, > > > >> the patch > > > >> is the best I can think of by preserving as much information as > > possible > > > >> from input > > > >> frames. I also understand it may break concept that "filter_frame" > > > outputs > > > >> one frame > > > >> at a time. For alimiter with my patch, depending on the size of the > > > >> lookahead buffer, > > > >> it may take a few frames before one output frame can be generated. > > This > > > is > > > >> inevitable > > > >> to compensate for the delay of the lookahead buffer. > > > >> > > > >> Thanks again for reviewing my patch and I'm looking forward to > hearing > > > >> from > > > >> you :) > > > >> > > > > > > > > Better than (because its no more 1 frame X nb_samples in, 1 frame X > > > > nb_samples out) to replace .filter_frame/.request_frame with > .activate > > > > logic. > > > > > > > > And make this output delay compensation filtering optional. > > > > > > > > In this process make sure that output PTS frame timestamps are > > unchanged > > > > from input one, by keeping reference of needed frames in filter > queue. > > > > > > > > Look how speechnorm/dynaudnorm does it. > > > > > > > > > > > > > Alternatively, use current logic in ladspa filter, (also add option > with > > > same name). > > > > > > This would need less code, and probably better approach, as there is no > > > extra latency introduced. > > > > > > Than mapping 1:1 between same number of samples per frame is not hold > any > > > more, but I think that is not much important any more. > > > > > Thank you for replying to me with your valuable feedback! I have checked > > af_ladspa > > and the "request_frame" function in af_ladspa looks similar to what I'm > > doing. The > > difference comes from the fact that I need an internal frame buffer to > keep > > track of > > output frames. Essentially I add a frame to the internal buffer when an > > input frame > > comes in. The frames in this buffer will be the future output frames. We >
