diff mbox

[FFmpeg-devel,2/3] lavf/avio: temporarily accept 0 as EOF.

Message ID 20171027184629.20772-2-george@nsup.org
State New
Headers show

Commit Message

Nicolas George Oct. 27, 2017, 6:46 p.m. UTC
Print a warning to let applicatios fix their use.
After a deprecation period, check with a low-level assert.
Also make the constraint explicit in the doxygen comment.

Signed-off-by: Nicolas George <george@nsup.org>
---
 libavformat/avio.h    |  2 ++
 libavformat/aviobuf.c | 30 +++++++++++++++++++++---------
 libavformat/version.h |  3 +++
 3 files changed, 26 insertions(+), 9 deletions(-)

Comments

Jan Ekström Oct. 27, 2017, 6:56 p.m. UTC | #1
On Fri, Oct 27, 2017 at 9:46 PM, Nicolas George <george@nsup.org> wrote:
> Print a warning to let applicatios fix their use.
> After a deprecation period, check with a low-level assert.
> Also make the constraint explicit in the doxygen comment.

Difference to the previous version:
> -        av_log(s, AV_LOG_WARNING, "Invalid return value 0 for stream protocol\n");
> +        av_log(NULL, AV_LOG_WARNING, "Invalid return value 0 for stream protocol\n");

Thus LGTM.

Jan
Jan Ekström Oct. 29, 2017, 5:46 p.m. UTC | #2
On Fri, Oct 27, 2017 at 9:56 PM, Jan Ekstrom <jeebjp@gmail.com> wrote:
> On Fri, Oct 27, 2017 at 9:46 PM, Nicolas George <george@nsup.org> wrote:
>> Print a warning to let applicatios fix their use.
>> After a deprecation period, check with a low-level assert.
>> Also make the constraint explicit in the doxygen comment.
>
> Difference to the previous version:
>> -        av_log(s, AV_LOG_WARNING, "Invalid return value 0 for stream protocol\n");
>> +        av_log(NULL, AV_LOG_WARNING, "Invalid return value 0 for stream protocol\n");
>
> Thus LGTM.
>
> Jan

The patch set has been applied.


Jan
diff mbox

Patch

diff --git a/libavformat/avio.h b/libavformat/avio.h
index 19ecd96eb7..76ff7cd81e 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -452,6 +452,8 @@  void avio_free_directory_entry(AVIODirEntry **entry);
  * @param write_flag Set to 1 if the buffer should be writable, 0 otherwise.
  * @param opaque An opaque pointer to user-specific data.
  * @param read_packet  A function for refilling the buffer, may be NULL.
+ *                     For stream protocols, must never return 0 but rather
+ *                     a proper AVERROR code.
  * @param write_packet A function for writing the buffer contents, may be NULL.
  *        The function may not change the input buffers content.
  * @param seek A function for seeking to specified byte position, may be NULL.
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 3e9d774a13..bfd40f5097 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -524,6 +524,24 @@  void avio_write_marker(AVIOContext *s, int64_t time, enum AVIODataMarkerType typ
     s->last_time = time;
 }
 
+static int read_packet_wrapper(AVIOContext *s, uint8_t *buf, int size)
+{
+    int ret;
+
+    if (!s->read_packet)
+        return AVERROR_EOF;
+    ret = s->read_packet(s->opaque, buf, size);
+#if FF_API_OLD_AVIO_EOF_0
+    if (!ret && !s->max_packet_size) {
+        av_log(NULL, AV_LOG_WARNING, "Invalid return value 0 for stream protocol\n");
+        ret = AVERROR_EOF;
+    }
+#else
+    av_assert2(ret || s->max_packet_size);
+#endif
+    return ret;
+}
+
 /* Input stream */
 
 static void fill_buffer(AVIOContext *s)
@@ -562,10 +580,7 @@  static void fill_buffer(AVIOContext *s)
         len = s->orig_buffer_size;
     }
 
-    if (s->read_packet)
-        len = s->read_packet(s->opaque, dst, len);
-    else
-        len = AVERROR_EOF;
+    len = read_packet_wrapper(s, dst, len);
     if (len == AVERROR_EOF) {
         /* do not modify buffer if EOF reached so that a seek back can
            be done without rereading data */
@@ -638,10 +653,7 @@  int avio_read(AVIOContext *s, unsigned char *buf, int size)
         if (len == 0 || s->write_flag) {
             if((s->direct || size > s->buffer_size) && !s->update_checksum) {
                 // bypass the buffer and read data directly into buf
-                if(s->read_packet)
-                    len = s->read_packet(s->opaque, buf, size);
-                else
-                    len = AVERROR_EOF;
+                len = read_packet_wrapper(s, buf, size);
                 if (len == AVERROR_EOF) {
                     /* do not modify buffer if EOF reached so that a seek back can
                     be done without rereading data */
@@ -708,7 +720,7 @@  int avio_read_partial(AVIOContext *s, unsigned char *buf, int size)
         return -1;
 
     if (s->read_packet && s->write_flag) {
-        len = s->read_packet(s->opaque, buf, size);
+        len = read_packet_wrapper(s, buf, size);
         if (len > 0)
             s->pos += len;
         return len;
diff --git a/libavformat/version.h b/libavformat/version.h
index ac409dbad2..0552f0c62c 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -76,6 +76,9 @@ 
 #ifndef FF_API_OLD_ROTATE_API
 #define FF_API_OLD_ROTATE_API           (LIBAVFORMAT_VERSION_MAJOR < 59)
 #endif
+#ifndef FF_API_OLD_AVIO_EOF_0
+#define FF_API_OLD_AVIO_EOF_0           (LIBAVFORMAT_VERSION_MAJOR < 59)
+#endif
 
 
 #ifndef FF_API_R_FRAME_RATE