| Message ID | 20220209152858.2339-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/setts_bsf: set the output packet duration to 0 | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/configure_loongarch64 | warning | Failed to run configure |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
On 2/9/2022 12:28 PM, James Almer wrote: > It's not possible to know the resulting packet's duration after > applying the expression as it depends on the timestamp of the next > packet, which we haven't seen yet. > The old duration, if any, is no longer valid, so just remove it. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/setts_bsf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavcodec/setts_bsf.c b/libavcodec/setts_bsf.c > index d604f91f71..56307fc8cd 100644 > --- a/libavcodec/setts_bsf.c > +++ b/libavcodec/setts_bsf.c > @@ -180,6 +180,7 @@ static int setts_filter(AVBSFContext *ctx, AVPacket *pkt) > > pkt->pts = new_pts; > pkt->dts = new_dts; > + pkt->duration = 0; > > return ret; > } Will apply.
Too soon. Wait more, does this breaks something?
On 2/13/2022 2:14 PM, Paul B Mahol wrote: > Too soon. > > Wait more, Ok. > does this breaks something? There are no fate tests, and 0 is used when the duration is unknown, so it shouldn't break anything. > _______________________________________________ > 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".
James Almer: > On 2/13/2022 2:14 PM, Paul B Mahol wrote: >> Too soon. >> >> Wait more, > > Ok. > >> does this breaks something? > > There are no fate tests, and 0 is used when the duration is unknown, so > it shouldn't break anything. > It breaks the simple use-case where this filter is used to just shift the timestamps. - Andreas
On 2/13/2022 2:22 PM, Andreas Rheinhardt wrote: > James Almer: >> On 2/13/2022 2:14 PM, Paul B Mahol wrote: >>> Too soon. >>> >>> Wait more, >> >> Ok. >> >>> does this breaks something? >> >> There are no fate tests, and 0 is used when the duration is unknown, so >> it shouldn't break anything. >> > > It breaks the simple use-case where this filter is used to just shift > the timestamps. How so? It's a documented value that means unknown. What breaks because of it? Wouldn't it be revealing a bug if so? And in non simple use cases that completely replace or rescale the timestamps, the old duration value is no longer valid, and something definitely worse to have than 0. > > - Andreas > _______________________________________________ > 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".
James Almer: > > > On 2/13/2022 2:22 PM, Andreas Rheinhardt wrote: >> James Almer: >>> On 2/13/2022 2:14 PM, Paul B Mahol wrote: >>>> Too soon. >>>> >>>> Wait more, >>> >>> Ok. >>> >>>> does this breaks something? >>> >>> There are no fate tests, and 0 is used when the duration is unknown, so >>> it shouldn't break anything. >>> >> >> It breaks the simple use-case where this filter is used to just shift >> the timestamps. > > How so? It's a documented value that means unknown. What breaks because > of it? Wouldn't it be revealing a bug if so? > Effectively forgetting a known value can have bad consequences. Think of subtitles. > And in non simple use cases that completely replace or rescale the > timestamps, the old duration value is no longer valid, and something > definitely worse to have than 0. > I am not saying that this filter is currently always outputting valid durations. - Andreas
On 2022-02-13 10:58 pm, James Almer wrote: > > > On 2/13/2022 2:22 PM, Andreas Rheinhardt wrote: >> James Almer: >>> On 2/13/2022 2:14 PM, Paul B Mahol wrote: >>>> Too soon. >>>> >>>> Wait more, >>> >>> Ok. >>> >>>> does this breaks something? >>> >>> There are no fate tests, and 0 is used when the duration is unknown, so >>> it shouldn't break anything. >>> >> >> It breaks the simple use-case where this filter is used to just shift >> the timestamps. > > How so? It's a documented value that means unknown. What breaks > because of it? Wouldn't it be revealing a bug if so? > > And in non simple use cases that completely replace or rescale the > timestamps, the old duration value is no longer valid, and something > definitely worse to have than 0. If the legacy value remains, a downstream processor can still use it for some reference or heuristic. And it can peek and compare adjacent timestamps to harmonize the duration value if needed. But once zeroed out, that hint is gone. Regards, Gyan
diff --git a/libavcodec/setts_bsf.c b/libavcodec/setts_bsf.c index d604f91f71..56307fc8cd 100644 --- a/libavcodec/setts_bsf.c +++ b/libavcodec/setts_bsf.c @@ -180,6 +180,7 @@ static int setts_filter(AVBSFContext *ctx, AVPacket *pkt) pkt->pts = new_pts; pkt->dts = new_dts; + pkt->duration = 0; return ret; }
It's not possible to know the resulting packet's duration after applying the expression as it depends on the timestamp of the next packet, which we haven't seen yet. The old duration, if any, is no longer valid, so just remove it. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/setts_bsf.c | 1 + 1 file changed, 1 insertion(+)