On 3/23/2021 3:47 PM, Nicolas George wrote:
James Almer (12021-03-23):
Signed-off-by: James Almer <jamr...@gmail.com>
---
Following "lavf: move AVStream.*index_entries* to AVStreamInternal", it was
revealed that some library users apparently found these fields useful and were
accessing them despite not being public.
This patch introduces a few functions to recover this functionality in a proper
and supported way.


We can't simply return a const pointer to the corresponding AVIndexEntry in the
array because it may change at any time by for example calling
av_add_index_entry(), so it's either a copy of the AVIndexEntry, or adding half
a dozen output parameters to the function signature for each field in
AVIndexEntry.

We can return a const pointer to the AVIndexEntry if we document it:

        Warning: the returned pointer is only valid until the next
        operation that may modify the AVStream or the AVFormatContext it
        belongs to. Accessing it after av_read_frame() or other
        functions results in undefined behavior.

Users can still copy it immediately if they need it longer.

I really don't like the idea of returning a pointer to some offset within an internal struct that may either start pointing at some other entry or even to freed memory, especially when the alternative of giving the user a copy of a single entry is trivial to do and actually safe.



  libavformat/avformat.h | 39 +++++++++++++++++++++++++++++++++++++++
  libavformat/utils.c    | 32 ++++++++++++++++++++++++++++++++
  2 files changed, 71 insertions(+)

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 765bc3b6f5..ac8b57149e 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -2754,6 +2754,45 @@ int av_find_default_stream_index(AVFormatContext *s);
   */
  int av_index_search_timestamp(AVStream *st, int64_t timestamp, int flags);
+/**
+ * Get the index entry count for the given AVStream.
+ *
+ * @param st stream
+ * @return the number of index entries in the stream
+ */
+int av_index_get_entries_count(AVStream *st);
+
+/**
+ * Get the AVIndexEntry corresponding to the given index.
+ *
+ * @param st          stream containing the requested AVIndexEntry
+ * @param index_entry pointer to an AVIndexEntry where to store the entry. On 
failure,
+ *                    or if the index entry could not be found, this struct 
will remain
+ *                    untouched
+ * @param idx         the desired index
+ * @return >= 0 on success. AVERROR(ENOENT) if no entry for the requested 
index could
+           be found. other negative AVERROR codes on failure.
+ */
+int av_index_get_entry(AVStream *st, AVIndexEntry *index_entry, int idx);
+
+/**
+ * Get the AVIndexEntry corresponding to the given timestamp.
+ *
+ * @param st          stream containing the requested AVIndexEntry
+ * @param index_entry pointer to an AVIndexEntry where to store the entry. On 
failure,
+ *                    or if the index entry could not be found, this struct 
will remain
+ *                    untouched
+ * @param timestamp   timestamp to retrieve the index entry for
+ * @param flags       if AVSEEK_FLAG_BACKWARD then the returned entry will 
correspond
+ *                    to the timestamp which is <= the requested one, if 
backward
+ *                    is 0, then it will be >=
+ *                    if AVSEEK_FLAG_ANY seek to any frame, only keyframes 
otherwise
+ * @return >= 0 on success. AVERROR(ENOENT) if no entry for the requested 
timestamp could
+ *          be found. other negative AVERROR codes on failure.
+ */
+int av_index_get_entry_from_timestamp(AVStream *st, AVIndexEntry *index_entry,
+                                      int64_t wanted_timestamp, int flags);
+
  /**
   * Add an index entry into a sorted list. Update the entry if the list
   * already contains it.
diff --git a/libavformat/utils.c b/libavformat/utils.c
index f31826f2ea..4e7a8bf23e 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -2129,6 +2129,38 @@ int av_index_search_timestamp(AVStream *st, int64_t 
wanted_timestamp, int flags)
                                       wanted_timestamp, flags);
  }
+int av_index_get_entries_count(AVStream *st)
+{
+    return st->internal->nb_index_entries;
+}
+
+int av_index_get_entry(AVStream *st, AVIndexEntry *index_entry, int idx)
+{

+    if (idx < 0)
+        return AVERROR(EINVAL);

Make idx unsigned.

Ok.


+    if (idx >= st->internal->nb_index_entries)
+        return AVERROR(ENOENT);
+

+    memcpy(index_entry, &st->internal->index_entries[idx], 
sizeof(*index_entry));

Why not using an assignment?

But this makes sizeof(AVIndexEntry) part of the ABI. Probably not what
we want.

The struct itself says nothing about it not being part of the ABI, and we usually document when that's the case to let the user know. That being said, no public function until now used that struct as parameter, so to not effectively tie it to the ABI if that's undesired, i could instead make index_entry a pointer to pointer, do an av_memdup(), and document that the user owns the returned entry and must free it after using it.


+
+    return 0;
+}
+
+int av_index_get_entry_from_timestamp(AVStream *st, AVIndexEntry *index_entry,
+                                      int64_t wanted_timestamp, int flags)
+{
+    int idx = ff_index_search_timestamp(st->internal->index_entries,
+                                        st->internal->nb_index_entries,
+                                        wanted_timestamp, flags);
+
+    if (idx < 0)
+        return AVERROR(ENOENT);
+

+    memcpy(index_entry, &st->internal->index_entries[idx], 
sizeof(*index_entry));

Same, of course.

+
+    return 0;
+}
+
  static int64_t ff_read_timestamp(AVFormatContext *s, int stream_index, 
int64_t *ppos, int64_t pos_limit,
                                   int64_t (*read_timestamp)(struct 
AVFormatContext *, int , int64_t *, int64_t ))
  {

Regards,


_______________________________________________
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".

Reply via email to