On Wed, Oct 25, 2017 at 11:22 AM, Nicolas George <geo...@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.
>
> Signed-off-by: Nicolas George <geo...@nsup.org>
Thanks for the patch.
> ---
> libavformat/avio.h | 2 ++
> libavformat/aviobuf.c | 30 +++++++++++++++++++++---------
> libavformat/version.h | 3 +++
> 3 files changed, 26 insertions(+), 9 deletions(-)
>
> 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.
Otherwise OK, but since this is the first mention of "stream protocol"
in the repo and el goog gives me just "Internet Stream Protocol" so I
would like an explanation of what this means and how it connects to
"custom AVIO implementations/wrappers"?
> * @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..bb5bcf7a14 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) {
Can you explain how you can utilize max_packet_size as a note
regarding if zero is EOF or not? Is it just a variable that happens to
be set to zero with custom AVIO contexts, or is there some other logic
behind this?
> + av_log(s, 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 0feb788c36..358ab91ab2 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -79,6 +79,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
> --
> 2.14.2
Otherwise LGTM. Will test in a moment.
Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
