Re: [FFmpeg-devel] [PATCH v4 2/3] avformat/imf: fix packet pts, dts and muxing

2022-02-17 Thread Zane van Iperen




On 17/2/22 02:54, p...@sandflow.com wrote:

  av_log(s,
 AV_LOG_DEBUG,
 "Switch resource on track %d: re-open context\n",
 track->index);
-if (open_track_resource_context(s, &(track->resources[i])) != 
0)
-return NULL;
+
+ret = open_track_resource_context(s, &(track->resources[i]));


Nit: "track->resources + i" is easier to read imo


+if (ret != 0)
+return ret;
  if (track->current_resource_index > 0)
  
avformat_close_input(>resources[track->current_resource_index].ctx);
  track->current_resource_index = i;
  }
  
-return &(track->resources[track->current_resource_index]);

+*resource = &(track->resources[track->current_resource_index]);


Nit: track->resources + track->current_resource_index



The rest lgtm.

Will apply (with nits fixed) in a few days of no objections.

___
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] [PATCH v4 2/3] avformat/imf: fix packet pts, dts and muxing

2022-02-16 Thread pal
From: Pierre-Anthony Lemieux 

The IMF demuxer does not set the DTS and PTS of packets accurately in all
scenarios. Moreover, audio packets are not trimmed when they exceed the
duration of the underlying resource.

imf-cpl-with-repeat FATE ref file is regenerated.

Addresses https://trac.ffmpeg.org/ticket/9611

---
 libavformat/imfdec.c   | 263 +++--
 tests/ref/fate/imf-cpl-with-repeat |  20 +--
 2 files changed, 181 insertions(+), 102 deletions(-)

diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
index 48bd5c78e8..17a21f5ef9 100644
--- a/libavformat/imfdec.c
+++ b/libavformat/imfdec.c
@@ -65,8 +65,10 @@
 #include "avio_internal.h"
 #include "imf.h"
 #include "internal.h"
+#include "libavcodec/packet.h"
 #include "libavutil/avstring.h"
 #include "libavutil/bprint.h"
+#include "libavutil/intreadwrite.h"
 #include "libavutil/opt.h"
 #include "mxf.h"
 #include "url.h"
@@ -97,6 +99,9 @@ typedef struct IMFVirtualTrackResourcePlaybackCtx {
 IMFAssetLocator *locator;
 FFIMFTrackFileResource *resource;
 AVFormatContext *ctx;
+AVRational start_time;
+AVRational end_time;
+AVRational ts_offset;
 } IMFVirtualTrackResourcePlaybackCtx;
 
 typedef struct IMFVirtualTrackPlaybackCtx {
@@ -108,7 +113,6 @@ typedef struct IMFVirtualTrackPlaybackCtx {
 IMFVirtualTrackResourcePlaybackCtx *resources; /**< Buffer holding the 
resources */
 int32_t current_resource_index;/**< Index of the current 
resource in resources,
 or < 0 if a current 
resource has yet to be selected */
-int64_t last_pts;  /**< Last timestamp */
 } IMFVirtualTrackPlaybackCtx;
 
 typedef struct IMFContext {
@@ -342,6 +346,7 @@ static int open_track_resource_context(AVFormatContext *s,
 int ret = 0;
 int64_t entry_point;
 AVDictionary *opts = NULL;
+AVStream *st;
 
 if (track_resource->ctx) {
 av_log(s,
@@ -383,23 +388,28 @@ static int open_track_resource_context(AVFormatContext *s,
 }
 av_dict_free();
 
-/* Compare the source timebase to the resource edit rate,
- * considering the first stream of the source file
- */
-if (av_cmp_q(track_resource->ctx->streams[0]->time_base,
- av_inv_q(track_resource->resource->base.edit_rate)))
+/* make sure there is only one stream in the file */
+
+if (track_resource->ctx->nb_streams != 1) {
+ret = AVERROR_INVALIDDATA;
+goto cleanup;
+}
+
+st = track_resource->ctx->streams[0];
+
+/* Warn if the resource time base does not match the file time base */
+if (av_cmp_q(st->time_base, 
av_inv_q(track_resource->resource->base.edit_rate)))
 av_log(s,
AV_LOG_WARNING,
-   "Incoherent source stream timebase %d/%d regarding resource 
edit rate: %d/%d",
-   track_resource->ctx->streams[0]->time_base.num,
-   track_resource->ctx->streams[0]->time_base.den,
+   "Incoherent source stream timebase " AVRATIONAL_FORMAT
+   "regarding resource edit rate: " AVRATIONAL_FORMAT,
+   st->time_base.num,
+   st->time_base.den,
track_resource->resource->base.edit_rate.den,
track_resource->resource->base.edit_rate.num);
 
-entry_point = (int64_t)track_resource->resource->base.entry_point
-* track_resource->resource->base.edit_rate.den
-* AV_TIME_BASE
-/ track_resource->resource->base.edit_rate.num;
+entry_point = av_rescale_q(track_resource->resource->base.entry_point, 
st->time_base,
+   
av_inv_q(track_resource->resource->base.edit_rate));
 
 if (entry_point) {
 av_log(s,
@@ -407,7 +417,7 @@ static int open_track_resource_context(AVFormatContext *s,
"Seek at resource %s entry point: %" PRIu32 "\n",
track_resource->locator->absolute_uri,
track_resource->resource->base.entry_point);
-ret = avformat_seek_file(track_resource->ctx, -1, entry_point, 
entry_point, entry_point, 0);
+ret = avformat_seek_file(track_resource->ctx, 0, entry_point, 
entry_point, entry_point, 0);
 if (ret < 0) {
 av_log(s,
AV_LOG_ERROR,
@@ -470,11 +480,16 @@ static int open_track_file_resource(AVFormatContext *s,
 vt_ctx.locator = asset_locator;
 vt_ctx.resource = track_file_resource;
 vt_ctx.ctx = NULL;
-track->resources[track->resource_count++] = vt_ctx;
-track->duration = av_add_q(track->duration,
+vt_ctx.start_time = track->duration;
+vt_ctx.ts_offset = av_sub_q(vt_ctx.start_time,
+
av_div_q(av_make_q((int)track_file_resource->base.entry_point, 1),
+ 
track_file_resource->base.edit_rate));
+vt_ctx.end_time = av_add_q(track->duration,