Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/5] lavf/rtpdec_asf: set AVFMT_FLAG_NONBLOCK
@ 2022-11-08 11:25 Anton Khirnov
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set Anton Khirnov
                   ` (3 more replies)
  0 siblings, 4 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-11-08 11:25 UTC (permalink / raw)
  To: ffmpeg-devel

Makes sure EAGAIN from the internal ASF demuxer is propagated to the RTP
demuxer. Will be important in following commits.
---
 libavformat/rtpdec_asf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavformat/rtpdec_asf.c b/libavformat/rtpdec_asf.c
index 72ead6975a..7824082d22 100644
--- a/libavformat/rtpdec_asf.c
+++ b/libavformat/rtpdec_asf.c
@@ -128,6 +128,7 @@ int ff_wms_parse_sdp_a_line(AVFormatContext *s, const char *p)
             return AVERROR(ENOMEM);
         }
         rt->asf_ctx->pb      = &pb.pub;
+        rt->asf_ctx->flags  |= AVFMT_FLAG_NONBLOCK;
         av_dict_set(&opts, "no_resync_search", "1", 0);
 
         if ((ret = ff_copy_whiteblacklists(rt->asf_ctx, s)) < 0) {
-- 
2.35.1

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-08 11:25 [FFmpeg-devel] [PATCH 1/5] lavf/rtpdec_asf: set AVFMT_FLAG_NONBLOCK Anton Khirnov
@ 2022-11-08 11:25 ` Anton Khirnov
  2022-11-10 12:28   ` Michael Niedermayer
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN) Anton Khirnov
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-08 11:25 UTC (permalink / raw)
  To: ffmpeg-devel

Lavf only supports a very limited approximation of non-blocking
behavior, so we should not return random EAGAINs to callers unless they
specifically requested it.
---
 libavformat/demux.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavformat/demux.c b/libavformat/demux.c
index 2dfd82a63c..5cd9522367 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -574,8 +574,12 @@ FF_ENABLE_DEPRECATION_WARNINGS
 
             /* Some demuxers return FFERROR_REDO when they consume
                data and discard it (ignored streams, junk, extradata).
-               We must re-call the demuxer to get the real packet. */
-            if (err == FFERROR_REDO)
+               We must re-call the demuxer to get the real packet.
+
+               Treat EAGAIN the same as FFERROR_REDO, unless the user
+               requested non-blocking behavior. */
+            if (err == FFERROR_REDO ||
+                (err == AVERROR(EAGAIN) && !(s->flags & AVFMT_FLAG_NONBLOCK)))
                 continue;
             if (!pktl || err == AVERROR(EAGAIN))
                 return err;
-- 
2.35.1

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-08 11:25 [FFmpeg-devel] [PATCH 1/5] lavf/rtpdec_asf: set AVFMT_FLAG_NONBLOCK Anton Khirnov
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set Anton Khirnov
@ 2022-11-08 11:25 ` Anton Khirnov
  2022-11-08 12:54   ` Nicolas George
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 4/5] fftools/ffmpeg_demux: do not set AVFMT_FLAG_NONBLOCK Anton Khirnov
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 5/5] fftools/ffmpeg: make demuxing with one file always blocking Anton Khirnov
  3 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-08 11:25 UTC (permalink / raw)
  To: ffmpeg-devel

There is no meaningful distinction between them, both mean "the demuxer
did some work, but did not produce a packet - should be called again".
---
 libavformat/demux.c   | 12 ++++--------
 libavformat/demux.h   |  6 ------
 libavformat/flvdec.c  | 14 +++++++-------
 libavformat/lxfdec.c  |  2 +-
 libavformat/mlvdec.c  |  2 +-
 libavformat/mpeg.c    |  2 +-
 libavformat/smacker.c |  2 +-
 7 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/libavformat/demux.c b/libavformat/demux.c
index 5cd9522367..8c81a58274 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -572,14 +572,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
         if (err < 0) {
             av_packet_unref(pkt);
 
-            /* Some demuxers return FFERROR_REDO when they consume
-               data and discard it (ignored streams, junk, extradata).
-               We must re-call the demuxer to get the real packet.
-
-               Treat EAGAIN the same as FFERROR_REDO, unless the user
-               requested non-blocking behavior. */
-            if (err == FFERROR_REDO ||
-                (err == AVERROR(EAGAIN) && !(s->flags & AVFMT_FLAG_NONBLOCK)))
+            /* Some demuxers return AVERROR(EAGAIN) when they consume
+               data and discard it (ignored streams, junk, extradata). Call the
+               demuxer again unless the user requested non-blocking behavior. */
+            if (err == AVERROR(EAGAIN) && !(s->flags & AVFMT_FLAG_NONBLOCK))
                 continue;
             if (!pktl || err == AVERROR(EAGAIN))
                 return err;
diff --git a/libavformat/demux.h b/libavformat/demux.h
index 1f57e062f6..a008c3dba1 100644
--- a/libavformat/demux.h
+++ b/libavformat/demux.h
@@ -55,12 +55,6 @@ typedef struct FFStreamInfo {
     int     fps_last_dts_idx;
 } FFStreamInfo;
 
-/**
- * Returned by demuxers to indicate that data was consumed but discarded
- * (ignored streams or junk data). The framework will re-call the demuxer.
- */
-#define FFERROR_REDO FFERRTAG('R','E','D','O')
-
 #define RELATIVE_TS_BASE (INT64_MAX - (1LL << 48))
 
 static av_always_inline int is_relative(int64_t ts)
diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index d83edff727..d320a431db 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -1058,7 +1058,7 @@ retry:
     }
 
     if (size == 0) {
-        ret = FFERROR_REDO;
+        ret = AVERROR(EAGAIN);
         goto leave;
     }
 
@@ -1110,13 +1110,13 @@ skip:
             av_log(s, AV_LOG_ERROR, "Unable to seek to the next packet\n");
             return AVERROR_INVALIDDATA;
         }
-        ret = FFERROR_REDO;
+        ret = AVERROR(EAGAIN);
         goto leave;
     }
 
     /* skip empty data packets */
     if (!size) {
-        ret = FFERROR_REDO;
+        ret = AVERROR(EAGAIN);
         goto leave;
     }
 
@@ -1160,7 +1160,7 @@ skip:
         (st->discard >= AVDISCARD_BIDIR && ((flags & FLV_VIDEO_FRAMETYPE_MASK) == FLV_FRAME_DISP_INTER && stream_type == FLV_STREAM_TYPE_VIDEO)) ||
          st->discard >= AVDISCARD_ALL) {
         avio_seek(s->pb, next, SEEK_SET);
-        ret = FFERROR_REDO;
+        ret = AVERROR(EAGAIN);
         goto leave;
     }
 
@@ -1273,7 +1273,7 @@ retry_duration:
             if (st->codecpar->extradata) {
                 if ((ret = flv_queue_extradata(flv, s->pb, stream_type, size)) < 0)
                     return ret;
-                ret = FFERROR_REDO;
+                ret = AVERROR(EAGAIN);
                 goto leave;
             }
             if ((ret = flv_get_extradata(s, st, size)) < 0)
@@ -1284,14 +1284,14 @@ retry_duration:
             if (st->codecpar->codec_id == AV_CODEC_ID_AAC && t && !strcmp(t->value, "Omnia A/XE"))
                 st->codecpar->extradata_size = 2;
 
-            ret = FFERROR_REDO;
+            ret = AVERROR(EAGAIN);
             goto leave;
         }
     }
 
     /* skip empty data packets */
     if (!size) {
-        ret = FFERROR_REDO;
+        ret = AVERROR(EAGAIN);
         goto leave;
     }
 
diff --git a/libavformat/lxfdec.c b/libavformat/lxfdec.c
index 8003ae98b7..bd28dfe270 100644
--- a/libavformat/lxfdec.c
+++ b/libavformat/lxfdec.c
@@ -307,7 +307,7 @@ static int lxf_read_packet(AVFormatContext *s, AVPacket *pkt)
     if (stream > 1) {
         av_log(s, AV_LOG_WARNING,
                "got packet with illegal stream index %"PRIu32"\n", stream);
-        return FFERROR_REDO;
+        return AVERROR(EAGAIN);
     }
 
     if (stream == 1 && s->nb_streams < 2) {
diff --git a/libavformat/mlvdec.c b/libavformat/mlvdec.c
index db3b77bb9b..d510787fcd 100644
--- a/libavformat/mlvdec.c
+++ b/libavformat/mlvdec.c
@@ -422,7 +422,7 @@ static int read_packet(AVFormatContext *avctx, AVPacket *pkt)
 
     pb = mlv->pb[sti->index_entries[index].size];
     if (!pb) {
-        ret = FFERROR_REDO;
+        ret = AVERROR(EAGAIN);
         goto next_packet;
     }
     avio_seek(pb, sti->index_entries[index].pos, SEEK_SET);
diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
index 864b08d8f8..f566ff74bd 100644
--- a/libavformat/mpeg.c
+++ b/libavformat/mpeg.c
@@ -251,7 +251,7 @@ redo:
         if (avio_feof(s->pb))
             return AVERROR_EOF;
         // FIXME we should remember header_state
-        return FFERROR_REDO;
+        return AVERROR(EAGAIN);
     }
 
     if (startcode == PACK_START_CODE)
diff --git a/libavformat/smacker.c b/libavformat/smacker.c
index 1d54e8e917..052b6d28d4 100644
--- a/libavformat/smacker.c
+++ b/libavformat/smacker.c
@@ -332,7 +332,7 @@ static int smacker_read_packet(AVFormatContext *s, AVPacket *pkt)
     }
 
     if (s->streams[smk->videoindex]->discard >= AVDISCARD_ALL) {
-        ret = FFERROR_REDO;
+        ret = AVERROR(EAGAIN);
         goto next_frame;
     }
     if (smk->frame_size >= INT_MAX/2) {
-- 
2.35.1

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [FFmpeg-devel] [PATCH 4/5] fftools/ffmpeg_demux: do not set AVFMT_FLAG_NONBLOCK
  2022-11-08 11:25 [FFmpeg-devel] [PATCH 1/5] lavf/rtpdec_asf: set AVFMT_FLAG_NONBLOCK Anton Khirnov
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set Anton Khirnov
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN) Anton Khirnov
@ 2022-11-08 11:25 ` Anton Khirnov
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 5/5] fftools/ffmpeg: make demuxing with one file always blocking Anton Khirnov
  3 siblings, 0 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-11-08 11:25 UTC (permalink / raw)
  To: ffmpeg-devel

This is pointless now that demuxing always runs in a separate thread.

Remove special handling for AVERROR(EAGAIN), which should never be
returned now.
---
 fftools/ffmpeg_demux.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index ee867cc15c..8ea5318148 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -254,11 +254,6 @@ static void *input_thread(void *arg)
         DemuxMsg msg = { NULL };
 
         ret = av_read_frame(f->ctx, pkt);
-
-        if (ret == AVERROR(EAGAIN)) {
-            av_usleep(10000);
-            continue;
-        }
         if (ret < 0) {
             if (d->loop) {
                 /* signal looping to the consumer thread */
@@ -917,7 +912,6 @@ int ifile_open(OptionsContext *o, const char *filename)
     ic->subtitle_codec_id  = subtitle_codec_name ? ic->subtitle_codec->id : AV_CODEC_ID_NONE;
     ic->data_codec_id      = data_codec_name     ? ic->data_codec->id     : AV_CODEC_ID_NONE;
 
-    ic->flags |= AVFMT_FLAG_NONBLOCK;
     if (o->bitexact)
         ic->flags |= AVFMT_FLAG_BITEXACT;
     ic->interrupt_callback = int_cb;
-- 
2.35.1

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [FFmpeg-devel] [PATCH 5/5] fftools/ffmpeg: make demuxing with one file always blocking
  2022-11-08 11:25 [FFmpeg-devel] [PATCH 1/5] lavf/rtpdec_asf: set AVFMT_FLAG_NONBLOCK Anton Khirnov
                   ` (2 preceding siblings ...)
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 4/5] fftools/ffmpeg_demux: do not set AVFMT_FLAG_NONBLOCK Anton Khirnov
@ 2022-11-08 11:25 ` Anton Khirnov
  3 siblings, 0 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-11-08 11:25 UTC (permalink / raw)
  To: ffmpeg-devel

---
 fftools/ffmpeg_demux.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 8ea5318148..b32bac217d 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -363,8 +363,9 @@ static int thread_start(Demuxer *d)
     if (d->thread_queue_size <= 0)
         d->thread_queue_size = (nb_input_files > 1 ? 8 : 1);
 
-    if (f->ctx->pb ? !f->ctx->pb->seekable :
-        strcmp(f->ctx->iformat->name, "lavfi"))
+    if (nb_input_files > 1 &&
+        (f->ctx->pb ? !f->ctx->pb->seekable :
+         strcmp(f->ctx->iformat->name, "lavfi")))
         d->non_blocking = 1;
     ret = av_thread_message_queue_alloc(&d->in_thread_queue,
                                         d->thread_queue_size, sizeof(DemuxMsg));
-- 
2.35.1

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN) Anton Khirnov
@ 2022-11-08 12:54   ` Nicolas George
  2022-11-08 14:07     ` Anton Khirnov
  0 siblings, 1 reply; 33+ messages in thread
From: Nicolas George @ 2022-11-08 12:54 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Anton Khirnov (12022-11-08):
> There is no meaningful distinction between them, both mean "the demuxer
> did some work, but did not produce a packet - should be called again".

NAK, there a difference in semantics: AVEROR(EAGAIN) is for when data is
not available for external reasons, typically network blocking,
AVERROR_REDO is for when data is available and the demuxer will read it
as soon as it is restarted, just to avoid having a loop in the demuxer.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-08 12:54   ` Nicolas George
@ 2022-11-08 14:07     ` Anton Khirnov
  2022-11-08 14:15       ` Nicolas George
  0 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-08 14:07 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Nicolas George (2022-11-08 13:54:53)
> Anton Khirnov (12022-11-08):
> > There is no meaningful distinction between them, both mean "the demuxer
> > did some work, but did not produce a packet - should be called again".
> 
> NAK, there a difference in semantics: AVEROR(EAGAIN) is for when data is
> not available for external reasons, typically network blocking,

This is false - there are zero demuxers returning EAGAIN due to network
blocking. Furthermore, lavf architecture fundamentally does not allow
generic non-blocking operations, so the most any demuxer can do is
return at a few specific points. And since demuxers are supposed to be
independent of the underlying IO protocol, they fundamentally cannot
know anything about network state.

So there really is no meaningful difference between REDO and EAGAIN.
Both are just an opportunity for the caller to do something else before
trying again. If we believe giving the caller this option is valuable,
then it's valuable in both these cases and it makes no sense to
distinguish between them.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-08 14:07     ` Anton Khirnov
@ 2022-11-08 14:15       ` Nicolas George
  2022-11-08 16:39         ` Anton Khirnov
  0 siblings, 1 reply; 33+ messages in thread
From: Nicolas George @ 2022-11-08 14:15 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Anton Khirnov (12022-11-08):
> This is false - there are zero demuxers returning EAGAIN due to network
> blocking.

There are devices who return EAGAIN due to device blocking.

> So there really is no meaningful difference between REDO and EAGAIN.

There is a difference: REDO is internal, EAGAIN is public.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-08 14:15       ` Nicolas George
@ 2022-11-08 16:39         ` Anton Khirnov
  2022-11-09  8:21           ` Nicolas George
  0 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-08 16:39 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Nicolas George (2022-11-08 15:15:39)
> Anton Khirnov (12022-11-08):
> > This is false - there are zero demuxers returning EAGAIN due to network
> > blocking.
> 
> There are devices who return EAGAIN due to device blocking.

Sure, and that's about it. And as I said before - there is no way to
tell when a device will be ready, so this is of very limited usefulness.

> > So there really is no meaningful difference between REDO and EAGAIN.
> 
> There is a difference: REDO is internal, EAGAIN is public.

That is not a meaningful difference. A meaningful difference would be
one that has actionable consequences.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-08 16:39         ` Anton Khirnov
@ 2022-11-09  8:21           ` Nicolas George
  2022-11-09  8:44             ` Paul B Mahol
  2022-11-09  9:31             ` Anton Khirnov
  0 siblings, 2 replies; 33+ messages in thread
From: Nicolas George @ 2022-11-09  8:21 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Anton Khirnov (12022-11-08):
> Sure, and that's about it. And as I said before - there is no way to
> tell when a device will be ready, so this is of very limited usefulness.

This is not true for many devices. Sorry to contradict you once again,
but you would know it if you had any experience working with devices.

> That is not a meaningful difference. A meaningful difference would be
> one that has actionable consequences.

Well, it has actionable consequences: if you treat EAGAIN like REDO you
introduce a busy wait, and if you treat REDO like EAGAIN you introduce a
significant slowness. Now that you make me mention it, I remember it was
precisely the reason I introduced REDO: to fix a slowness in resyncs.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-09  8:21           ` Nicolas George
@ 2022-11-09  8:44             ` Paul B Mahol
  2022-11-09  9:21               ` Nicolas George
  2022-11-09  9:31             ` Anton Khirnov
  1 sibling, 1 reply; 33+ messages in thread
From: Paul B Mahol @ 2022-11-09  8:44 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On 11/9/22, Nicolas George <george@nsup.org> wrote:
> Anton Khirnov (12022-11-08):
>> Sure, and that's about it. And as I said before - there is no way to
>> tell when a device will be ready, so this is of very limited usefulness.
>
> This is not true for many devices. Sorry to contradict you once again,
> but you would know it if you had any experience working with devices.
>
>> That is not a meaningful difference. A meaningful difference would be
>> one that has actionable consequences.
>
> Well, it has actionable consequences: if you treat EAGAIN like REDO you
> introduce a busy wait, and if you treat REDO like EAGAIN you introduce a
> significant slowness. Now that you make me mention it, I remember it was
> precisely the reason I introduced REDO: to fix a slowness in resyncs.
>


I'm not interested in REDO / EAGAIN differences.
Can this bug be fixed at all?

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-09  8:44             ` Paul B Mahol
@ 2022-11-09  9:21               ` Nicolas George
  2022-11-09  9:23                 ` Paul B Mahol
  0 siblings, 1 reply; 33+ messages in thread
From: Nicolas George @ 2022-11-09  9:21 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Paul B Mahol (12022-11-09):
> I'm not interested in REDO / EAGAIN differences.

They delete this thread.

> Can this bug be fixed at all?

This bug has been fixed for seven years. I would rather we do not unfix
it.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-09  9:21               ` Nicolas George
@ 2022-11-09  9:23                 ` Paul B Mahol
  2022-11-09  9:29                   ` Nicolas George
  0 siblings, 1 reply; 33+ messages in thread
From: Paul B Mahol @ 2022-11-09  9:23 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On 11/9/22, Nicolas George <george@nsup.org> wrote:
> Paul B Mahol (12022-11-09):
>> I'm not interested in REDO / EAGAIN differences.
>
> They delete this thread.
>
>> Can this bug be fixed at all?
>
> This bug has been fixed for seven years. I would rather we do not unfix
> it.

The bug is found by Jan IIRC, and fixed by not using EAGAIN return
values in demuxer IIRC.

It would be better that bug is explained in commit log too.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-09  9:23                 ` Paul B Mahol
@ 2022-11-09  9:29                   ` Nicolas George
  0 siblings, 0 replies; 33+ messages in thread
From: Nicolas George @ 2022-11-09  9:29 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Paul B Mahol (12022-11-09):
> The bug is found by Jan IIRC, and fixed by not using EAGAIN return
> values in demuxer IIRC.

Then I do not know what bug you are referring to. Maybe try honing your
communication skills.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-09  8:21           ` Nicolas George
  2022-11-09  8:44             ` Paul B Mahol
@ 2022-11-09  9:31             ` Anton Khirnov
  2022-11-09  9:38               ` Nicolas George
  1 sibling, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-09  9:31 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Nicolas George (2022-11-09 09:21:34)
> Anton Khirnov (12022-11-08):
> > Sure, and that's about it. And as I said before - there is no way to
> > tell when a device will be ready, so this is of very limited usefulness.
> 
> This is not true for many devices. Sorry to contradict you once again,
> but you would know it if you had any experience working with devices.

We could really do with fewer of these personal attacks.

> > That is not a meaningful difference. A meaningful difference would be
> > one that has actionable consequences.
> 
> Well, it has actionable consequences: if you treat EAGAIN like REDO you
> introduce a busy wait,

In most devices it's a sleep rather than a busy wait. And in those where
it isn't, it should be.

Furthermore, since the caller has no way of knowing how long to wait,
there is little they can do other than sleeping for a random period and
hoping for the best.

> and if you treat REDO like EAGAIN you introduce a
> significant slowness. Now that you make me mention it, I remember it was
> precisely the reason I introduced REDO: to fix a slowness in resyncs.

I highly doubt that returning control back to the caller will cause any
slowdown in and of itself, it's more about what the caller will do in
response. If they choose to sleep for a random amount of time, then
maybe they should stop doing that (which is exactly what this patchset
does).

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-09  9:31             ` Anton Khirnov
@ 2022-11-09  9:38               ` Nicolas George
  2022-11-09 10:17                 ` Anton Khirnov
  0 siblings, 1 reply; 33+ messages in thread
From: Nicolas George @ 2022-11-09  9:38 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Anton Khirnov (12022-11-09):
> > Well, it has actionable consequences: if you treat EAGAIN like REDO you
> > introduce a busy wait,
> In most devices it's a sleep rather than a busy wait. And in those where
> it isn't, it should be.

I do not know how your sentence connects to mine.

> Furthermore, since the caller has no way of knowing how long to wait,
> there is little they can do other than sleeping for a random period and
> hoping for the best.

This is why I have wanted to fix the design of demuxers and demuxer-like
components for years, but it is a tremendous work. In the meantime, we
just do with "av_usleep(1000)" and it is terrible.

> I highly doubt that returning control back to the caller will cause any
> slowdown in and of itself, it's more about what the caller will do in
> response. If they choose to sleep for a random amount of time, then
> maybe they should stop doing that (which is exactly what this patchset
> does).

Sleeping is the only correct reaction to EAGAIN.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-09  9:38               ` Nicolas George
@ 2022-11-09 10:17                 ` Anton Khirnov
  2022-11-09 10:41                   ` Nicolas George
  0 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-09 10:17 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Nicolas George (2022-11-09 10:38:06)
> Anton Khirnov (12022-11-09):
> > > Well, it has actionable consequences: if you treat EAGAIN like REDO you
> > > introduce a busy wait,
> > In most devices it's a sleep rather than a busy wait. And in those where
> > it isn't, it should be.
> 
> I do not know how your sentence connects to mine.

If your concern with busy-waiting is pointless energy consumption, then
the correct thing to do is change all busy-waiting devices to sleep
internally if AVFMT_FLAG_NONBLOCK is not specified. I just checked, and
almost all of them actually do exactly this, the only exception is
avfoundation.

> 
> > Furthermore, since the caller has no way of knowing how long to wait,
> > there is little they can do other than sleeping for a random period and
> > hoping for the best.
> 
> This is why I have wanted to fix the design of demuxers and demuxer-like
> components for years, but it is a tremendous work. In the meantime, we
> just do with "av_usleep(1000)" and it is terrible.
> 
> > I highly doubt that returning control back to the caller will cause any
> > slowdown in and of itself, it's more about what the caller will do in
> > response. If they choose to sleep for a random amount of time, then
> > maybe they should stop doing that (which is exactly what this patchset
> > does).
> 
> Sleeping is the only correct reaction to EAGAIN.

First, this would be an incredibly strong claim - given how many
possible usage patterns there are - even if we did have support for
user-side polling. But since we do not, then it's even more dubious,
because the user does not know how long to sleep for. The only truly
correct way to work with arbitrary demuxers currently is run it in
blocking mode in a separate thread (which is exactly what ffmpeg.c does
now).

Furthermore this claim is not supported by development history. mpegts
will currently return EAGAIN on failed resyncs, specifically to give the
caller the opportunity to decide what to do next. RTSP does something
similar. This has nothing to do with waiting for the network, because
neither of these two demuxers know about the state of the network. And
there is no essential difference between what mpegts does with EAGAIN
and e.g. flvdec with REDO.

The point of this set is giving control to the caller. If the caller
wants blocking operations, they unset AVFMT_FLAG_NONBLOCK and don't have
to deal with EAGAINs. If they want the limited support for context
switching that lavf can provide, they set AVFMT_FLAG_NONBLOCK and decide
what to do.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-09 10:17                 ` Anton Khirnov
@ 2022-11-09 10:41                   ` Nicolas George
  2022-11-09 11:53                     ` Anton Khirnov
  0 siblings, 1 reply; 33+ messages in thread
From: Nicolas George @ 2022-11-09 10:41 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Anton Khirnov (12022-11-09):
> If your concern with busy-waiting is pointless energy consumption, then
> the correct thing to do is change all busy-waiting devices to sleep
> internally if AVFMT_FLAG_NONBLOCK is not specified. I just checked, and
> almost all of them actually do exactly this, the only exception is
> avfoundation.

Yes, devices currently do the right thing as much as our framework
permits.

Breaking that is one of the two ways merging EAGAIN and REDO can break
our code.

> First, this would be an incredibly strong claim - given how many
> possible usage patterns there are

Mostly true nonetheless.

>							The only truly
> correct way to work with arbitrary demuxers currently is run it in
> blocking mode in a separate thread (which is exactly what ffmpeg.c does
> now).

It is not a correct way, only slightly less broken.

> Furthermore this claim is not supported by development history. mpegts
> will currently return EAGAIN on failed resyncs, specifically to give the
> caller the opportunity to decide what to do next.

IIRC, this is precisely what REDO was introduced to fix. If the bug was
introduced again or incompletely fixed, this it is an issue. Indeed, the
two uses of EAGAIN in libavformat/mpegts.c seems highly dubious.

But as I explained, REDO and EAGAIN have a very different semantic with
regard to when the caller needs to retry and cannot be merged.

Now, if you want me to take from my time to explain it again with more
details and more clarity for your benefit… I suggest you tone done the
condescension towards me.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-09 10:41                   ` Nicolas George
@ 2022-11-09 11:53                     ` Anton Khirnov
  2022-11-09 12:42                       ` Nicolas George
  0 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-09 11:53 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Nicolas George (2022-11-09 11:41:32)
> Anton Khirnov (12022-11-09):
> > If your concern with busy-waiting is pointless energy consumption, then
> > the correct thing to do is change all busy-waiting devices to sleep
> > internally if AVFMT_FLAG_NONBLOCK is not specified. I just checked, and
> > almost all of them actually do exactly this, the only exception is
> > avfoundation.
> 
> Yes, devices currently do the right thing as much as our framework
> permits.
> 
> Breaking that is one of the two ways merging EAGAIN and REDO can break
> our code.

I have no idea what you mean by this. What is being broken and how?

> > Furthermore this claim is not supported by development history. mpegts
> > will currently return EAGAIN on failed resyncs, specifically to give the
> > caller the opportunity to decide what to do next.
> 
> IIRC, this is precisely what REDO was introduced to fix. If the bug was
> introduced again or incompletely fixed, this it is an issue. Indeed, the
> two uses of EAGAIN in libavformat/mpegts.c seems highly dubious.
> 
> But as I explained, REDO and EAGAIN have a very different semantic with
> regard to when the caller needs to retry and cannot be merged.

Maybe more context will clarify the motivation here. What became this
patchset started as an attempt to make all uses of EAGAIN/REDO in
lavf/lavd consistent. While doing that I gradually came to the
conclusion that the distinction is arbitrary - e.g. multiple demuxers
deliberately return EAGAIN as context switch method. Now we could
declare this practice invalid, but as this has been done in multiple
demuxers independently, and has been in the tree for over 10 years, so
this should not be done lightly.

The question then is - what should be the effective rule for deciding
whether a demuxer returns EAGAIN or REDO in any given case?
Opinions on this are very much welcome.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN)
  2022-11-09 11:53                     ` Anton Khirnov
@ 2022-11-09 12:42                       ` Nicolas George
  0 siblings, 0 replies; 33+ messages in thread
From: Nicolas George @ 2022-11-09 12:42 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Anton Khirnov (12022-11-09):
> I have no idea what you mean by this. What is being broken and how?

EAGAIN and REDO are different. If you make them the same, you break one
of them. Or possibly both if you are being really stupid, but at least
one.

> Maybe more context will clarify the motivation here. What became this
> patchset started as an attempt to make all uses of EAGAIN/REDO in
> lavf/lavd consistent.

And as I explained, it is not possible.

>			While doing that I gradually came to the
> conclusion that the distinction is arbitrary - e.g. multiple demuxers
> deliberately return EAGAIN as context switch method. Now we could
> declare this practice invalid, but as this has been done in multiple
> demuxers independently, and has been in the tree for over 10 years, so
> this should not be done lightly.

I introduced REDO precisely to fix demuxers who abused EAGAIN like that.

> The question then is - what should be the effective rule for deciding
> whether a demuxer returns EAGAIN or REDO in any given case?
> Opinions on this are very much welcome.

I do not have an opinion, I have knowledge.

The semantic of EAGAIN has always been, since it stabilized, that the
operation cannot be completed immediately, it requires an external
event, like a network packet or an IRQ on a device. Therefore, the only
correct reaction to EAGAIN is to delay retrying. Ideally delay until an
event is received; if that cannot be achieved a small arbitrary delay is
still infinitely better than busy wait.

This is standard Unix system and network programming, knowing this in
and out is IMO an absolute prerequisite for tinkering with FFmpeg's
network code.

The semantic of AVERROR(EAGAIN) is ours to decide, but it would be
really really stupid from us to decide for something that is not aligned
with the wider semantic of EAGAIN. And aligning with EAGAIN is what we
did, with the caveat that most of it only works with devices and
elementary protocols. Except for the bugs at hand.

Now, as you observed, some demuxers have abused AVERROR(EAGAIN) to let
the framework re-call them and continue their work, for example looking
for a resync marker.

This is an abuse, this is invalid. If AVERROR(EAGAIN) is handled
correctly, using it to look for a resync marker causes a delay to be
inserted. The delay is very small, negligible in most cases, but it
should not be there. And if the resync marker is hard to find, the delay
will add up.

If I remember correctly, this is precisely the issue AVERROR_REDO fixes:
a damaged sample that would be very slow to demux without consuming CPU
because of the delays introduced by the correct handling of
AVERROR(EAGAIN).

There was another way to fix the issue: have the demuxers loop
themselves looking for a resync. But it would have required more
reworking of the code.

Last detail: we do not have the API to return to the user in the middle
of the search for a resync marker. If this is something you want for
some reason, you will need to introduce something new. But if so, please
first explain your use case.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set Anton Khirnov
@ 2022-11-10 12:28   ` Michael Niedermayer
  2022-11-10 12:31     ` Anton Khirnov
  0 siblings, 1 reply; 33+ messages in thread
From: Michael Niedermayer @ 2022-11-10 12:28 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 688 bytes --]

On Tue, Nov 08, 2022 at 12:25:47PM +0100, Anton Khirnov wrote:
> Lavf only supports a very limited approximation of non-blocking
> behavior, so we should not return random EAGAINs to callers unless they
> specifically requested it.
> ---
>  libavformat/demux.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

this changes seeking behavior of for example:
libavformat/tests/seek fate-suite/dss/sp.dss

(this seemed to return EAGAIN before sometimes)

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-10 12:28   ` Michael Niedermayer
@ 2022-11-10 12:31     ` Anton Khirnov
  2022-11-10 12:52       ` Paul B Mahol
  2022-11-10 19:52       ` Michael Niedermayer
  0 siblings, 2 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-11-10 12:31 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Michael Niedermayer (2022-11-10 13:28:27)
> On Tue, Nov 08, 2022 at 12:25:47PM +0100, Anton Khirnov wrote:
> > Lavf only supports a very limited approximation of non-blocking
> > behavior, so we should not return random EAGAINs to callers unless they
> > specifically requested it.
> > ---
> >  libavformat/demux.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> this changes seeking behavior of for example:
> libavformat/tests/seek fate-suite/dss/sp.dss
> 
> (this seemed to return EAGAIN before sometimes)

Are you saying that is wrong? I don't see why seeks should return EAGAIN
either.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-10 12:31     ` Anton Khirnov
@ 2022-11-10 12:52       ` Paul B Mahol
  2022-11-10 13:33         ` Anton Khirnov
  2022-11-10 19:52       ` Michael Niedermayer
  1 sibling, 1 reply; 33+ messages in thread
From: Paul B Mahol @ 2022-11-10 12:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On 11/10/22, Anton Khirnov <anton@khirnov.net> wrote:
> Quoting Michael Niedermayer (2022-11-10 13:28:27)
>> On Tue, Nov 08, 2022 at 12:25:47PM +0100, Anton Khirnov wrote:
>> > Lavf only supports a very limited approximation of non-blocking
>> > behavior, so we should not return random EAGAINs to callers unless they
>> > specifically requested it.
>> > ---
>> >  libavformat/demux.c | 8 ++++++--
>> >  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> this changes seeking behavior of for example:
>> libavformat/tests/seek fate-suite/dss/sp.dss
>>
>> (this seemed to return EAGAIN before sometimes)
>
> Are you saying that is wrong? I don't see why seeks should return EAGAIN
> either.
>

It returns internally when passing seeking to generic seeking.
Please  do not break seeking in demuxers.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-10 12:52       ` Paul B Mahol
@ 2022-11-10 13:33         ` Anton Khirnov
  0 siblings, 0 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-11-10 13:33 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Paul B Mahol (2022-11-10 13:52:52)
> On 11/10/22, Anton Khirnov <anton@khirnov.net> wrote:
> > Quoting Michael Niedermayer (2022-11-10 13:28:27)
> >> On Tue, Nov 08, 2022 at 12:25:47PM +0100, Anton Khirnov wrote:
> >> > Lavf only supports a very limited approximation of non-blocking
> >> > behavior, so we should not return random EAGAINs to callers unless they
> >> > specifically requested it.
> >> > ---
> >> >  libavformat/demux.c | 8 ++++++--
> >> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> this changes seeking behavior of for example:
> >> libavformat/tests/seek fate-suite/dss/sp.dss
> >>
> >> (this seemed to return EAGAIN before sometimes)
> >
> > Are you saying that is wrong? I don't see why seeks should return EAGAIN
> > either.
> >
> 
> It returns internally when passing seeking to generic seeking.
> Please  do not break seeking in demuxers.

What exact use case is being broken by this?

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-10 12:31     ` Anton Khirnov
  2022-11-10 12:52       ` Paul B Mahol
@ 2022-11-10 19:52       ` Michael Niedermayer
  2022-11-14  9:23         ` Anton Khirnov
  1 sibling, 1 reply; 33+ messages in thread
From: Michael Niedermayer @ 2022-11-10 19:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1233 bytes --]

On Thu, Nov 10, 2022 at 01:31:44PM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2022-11-10 13:28:27)
> > On Tue, Nov 08, 2022 at 12:25:47PM +0100, Anton Khirnov wrote:
> > > Lavf only supports a very limited approximation of non-blocking
> > > behavior, so we should not return random EAGAINs to callers unless they
> > > specifically requested it.
> > > ---
> > >  libavformat/demux.c | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > this changes seeking behavior of for example:
> > libavformat/tests/seek fate-suite/dss/sp.dss
> > 
> > (this seemed to return EAGAIN before sometimes)
> 
> Are you saying that is wrong? I don't see why seeks should return EAGAIN
> either.

I do not know if its wrong or not. I remember some seeking used error
returns to switch to generic seeking and such stuff.
I just reporrted this as i saw a difference in the tests/seek output

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-10 19:52       ` Michael Niedermayer
@ 2022-11-14  9:23         ` Anton Khirnov
  2022-11-14 10:44           ` Nicolas George
  0 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-14  9:23 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Michael Niedermayer (2022-11-10 20:52:33)
> On Thu, Nov 10, 2022 at 01:31:44PM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2022-11-10 13:28:27)
> > > On Tue, Nov 08, 2022 at 12:25:47PM +0100, Anton Khirnov wrote:
> > > > Lavf only supports a very limited approximation of non-blocking
> > > > behavior, so we should not return random EAGAINs to callers unless they
> > > > specifically requested it.
> > > > ---
> > > >  libavformat/demux.c | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > this changes seeking behavior of for example:
> > > libavformat/tests/seek fate-suite/dss/sp.dss
> > > 
> > > (this seemed to return EAGAIN before sometimes)
> > 
> > Are you saying that is wrong? I don't see why seeks should return EAGAIN
> > either.
> 
> I do not know if its wrong or not. I remember some seeking used error
> returns to switch to generic seeking and such stuff.
> I just reporrted this as i saw a difference in the tests/seek output

AFAICT the differences are only in av_read_frame() calls, which no
longer return EAGAIN. That is exactly what this patch is supposed to do,
so I'd say there is no problem.

If noone has further objections, I'll push this patch soonish.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-14  9:23         ` Anton Khirnov
@ 2022-11-14 10:44           ` Nicolas George
  2022-11-14 11:00             ` Anton Khirnov
  0 siblings, 1 reply; 33+ messages in thread
From: Nicolas George @ 2022-11-14 10:44 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Anton Khirnov (12022-11-14):
> If noone has further objections, I'll push this patch soonish.

You will do no scu thing since it is bogus, as I explained.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-14 10:44           ` Nicolas George
@ 2022-11-14 11:00             ` Anton Khirnov
  2022-11-14 11:05               ` Nicolas George
  0 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-14 11:00 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Nicolas George (2022-11-14 11:44:55)
> Anton Khirnov (12022-11-14):
> > If noone has further objections, I'll push this patch soonish.
> 
> You will do no scu thing since it is bogus, as I explained.

I do not see any objections from you to this patch.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-14 11:00             ` Anton Khirnov
@ 2022-11-14 11:05               ` Nicolas George
  2022-11-14 11:12                 ` Anton Khirnov
  0 siblings, 1 reply; 33+ messages in thread
From: Nicolas George @ 2022-11-14 11:05 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Anton Khirnov (12022-11-14):
> I do not see any objections from you to this patch.

My explanations for the other patch apply to this one, obviously: if you
treat EAGAIN as REDO, you introduce a busy wait.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-14 11:05               ` Nicolas George
@ 2022-11-14 11:12                 ` Anton Khirnov
  2022-11-14 12:40                   ` Nicolas George
  0 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-14 11:12 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Nicolas George (2022-11-14 12:05:10)
> Anton Khirnov (12022-11-14):
> > I do not see any objections from you to this patch.
> 
> My explanations for the other patch apply to this one, obviously: if you
> treat EAGAIN as REDO, you introduce a busy wait.

In what specific case would this supposed busy wait happen and why is it
wrong?

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-14 11:12                 ` Anton Khirnov
@ 2022-11-14 12:40                   ` Nicolas George
  2022-11-14 16:17                     ` Anton Khirnov
  0 siblings, 1 reply; 33+ messages in thread
From: Nicolas George @ 2022-11-14 12:40 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Anton Khirnov (12022-11-14):
> In what specific case would this supposed busy wait happen and why is it
> wrong?

See my mail from 9 Nov 2022 13:42:24 +0100 for explanations about the
semantic of EAGAIN.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-14 12:40                   ` Nicolas George
@ 2022-11-14 16:17                     ` Anton Khirnov
  2022-11-14 16:22                       ` Nicolas George
  0 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-11-14 16:17 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Nicolas George (2022-11-14 13:40:35)
> Anton Khirnov (12022-11-14):
> > In what specific case would this supposed busy wait happen and why is it
> > wrong?
> 
> See my mail from 9 Nov 2022 13:42:24 +0100 for explanations about the
> semantic of EAGAIN.

You wrote a whole lot of text and I don't see how any of it applies to
this patch.

The standard convention everywhere is that IO may return EAGAIN in
non-blocking mode and blocks otherwise. And that is exactly what this
patch implements.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set
  2022-11-14 16:17                     ` Anton Khirnov
@ 2022-11-14 16:22                       ` Nicolas George
  0 siblings, 0 replies; 33+ messages in thread
From: Nicolas George @ 2022-11-14 16:22 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Anton Khirnov (12022-11-14):
> You wrote a whole lot of text and I don't see how any of it applies to
> this patch.
> 
> The standard convention everywhere is that IO may return EAGAIN in
> non-blocking mode and blocks otherwise. And that is exactly what this
> patch implements.

To implement blocking on top of EAGAIN, a sleep is necessary.

REDO must not sleep.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2022-11-14 16:22 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-08 11:25 [FFmpeg-devel] [PATCH 1/5] lavf/rtpdec_asf: set AVFMT_FLAG_NONBLOCK Anton Khirnov
2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 2/5] lavf/demux: treat EAGAIN as REDO unless AVFMT_FLAG_NONBLOCK is set Anton Khirnov
2022-11-10 12:28   ` Michael Niedermayer
2022-11-10 12:31     ` Anton Khirnov
2022-11-10 12:52       ` Paul B Mahol
2022-11-10 13:33         ` Anton Khirnov
2022-11-10 19:52       ` Michael Niedermayer
2022-11-14  9:23         ` Anton Khirnov
2022-11-14 10:44           ` Nicolas George
2022-11-14 11:00             ` Anton Khirnov
2022-11-14 11:05               ` Nicolas George
2022-11-14 11:12                 ` Anton Khirnov
2022-11-14 12:40                   ` Nicolas George
2022-11-14 16:17                     ` Anton Khirnov
2022-11-14 16:22                       ` Nicolas George
2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN) Anton Khirnov
2022-11-08 12:54   ` Nicolas George
2022-11-08 14:07     ` Anton Khirnov
2022-11-08 14:15       ` Nicolas George
2022-11-08 16:39         ` Anton Khirnov
2022-11-09  8:21           ` Nicolas George
2022-11-09  8:44             ` Paul B Mahol
2022-11-09  9:21               ` Nicolas George
2022-11-09  9:23                 ` Paul B Mahol
2022-11-09  9:29                   ` Nicolas George
2022-11-09  9:31             ` Anton Khirnov
2022-11-09  9:38               ` Nicolas George
2022-11-09 10:17                 ` Anton Khirnov
2022-11-09 10:41                   ` Nicolas George
2022-11-09 11:53                     ` Anton Khirnov
2022-11-09 12:42                       ` Nicolas George
2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 4/5] fftools/ffmpeg_demux: do not set AVFMT_FLAG_NONBLOCK Anton Khirnov
2022-11-08 11:25 ` [FFmpeg-devel] [PATCH 5/5] fftools/ffmpeg: make demuxing with one file always blocking Anton Khirnov

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git