Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[wm4]

On Tue, 18 Apr 2017 16:01:48 +0200
Michael Niedermayer  wrote:

> On Tue, Apr 18, 2017 at 03:47:29PM +0200, Nicolas George wrote:
> > Le nonidi 29 germinal, an CCXXV, Michael Niedermayer a écrit :  
> > > This contradicts the documentation: (and would be rather rigid design)  
> > 
> > Possible. But it has nothing to do with threading.
> >   
> > > > threaded decoder will just start the work and return EAGAIN.  
> > > Thats true but this is not permitted by the text prior to my patch  
> > 
> > Yes it is.
> >   
> 
> > > The requiement to call avcodec_receive_frame() multiple times implies
> > > it does not return EAGAIN because you would not call it again if it
> > > did.  
> > 
> > No, you would not call it again, you would first have to feed it another
> > packet. Still no problem at this level.  
> 
> Iam not sure you did read the text the patch changes
> 
> "... require you to call avcodec_receive_frame() multiple
>  times afterwards before you can send a new packet"
> 
> this just isnt true
> if a decoder returns EAGAIN you would not call it multiple times
> 

The decoder is free to do whatever it pleases. It _could_ force the API
user to "receive" multiple frames immediately, or it could buffer
multiple packets/frames internally and make the user "receive" them at
a later point.

The whole point of the API is to make this data flow more flexible. The
whole point about the sentence your patch changes is that you do not
somehow feed parts of the previous packet to new "send" invocations,
like the old audio decode API did.

Feel free to improve the docs (they surely are not perfect), but I
don't think your patch does. It seems to sort of miss the point.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[Michael Niedermayer]

On Tue, Apr 18, 2017 at 04:08:43PM +0200, Nicolas George wrote:
> Le nonidi 29 germinal, an CCXXV, Michael Niedermayer a écrit :
> > Iam not sure you did read the text the patch changes
> 
> I am replying to the issue about threading.

ok, iam trying to stay with the topic of this thread which was fixing
the documentation touched by the patch.

> 
> If you find the documentation inconsistent, your commit message must be
> changed.

Sure, what commit message would you suggest ?

Thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[Nicolas George]

Le nonidi 29 germinal, an CCXXV, Michael Niedermayer a écrit :
> Iam not sure you did read the text the patch changes

I am replying to the issue about threading.

If you find the documentation inconsistent, your commit message must be
changed.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[Michael Niedermayer]

On Tue, Apr 18, 2017 at 03:47:29PM +0200, Nicolas George wrote:
> Le nonidi 29 germinal, an CCXXV, Michael Niedermayer a écrit :
> > This contradicts the documentation: (and would be rather rigid design)
> 
> Possible. But it has nothing to do with threading.
> 
> > > threaded decoder will just start the work and return EAGAIN.
> > Thats true but this is not permitted by the text prior to my patch
> 
> Yes it is.
> 

> > The requiement to call avcodec_receive_frame() multiple times implies
> > it does not return EAGAIN because you would not call it again if it
> > did.
> 
> No, you would not call it again, you would first have to feed it another
> packet. Still no problem at this level.

Iam not sure you did read the text the patch changes

"... require you to call avcodec_receive_frame() multiple
 times afterwards before you can send a new packet"

this just isnt true
if a decoder returns EAGAIN you would not call it multiple times


Heres the whole again for reference:

  *  Unlike with older APIs, the packet is always fully 
consumed,
  *  and if it contains multiple frames (e.g. some audio 
codecs),
  *  will require you to call avcodec_receive_frame() multiple
- *  times afterwards before you can send a new packet.
+ *  times afterwards.

[...]

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


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[Nicolas George]

Le nonidi 29 germinal, an CCXXV, Michael Niedermayer a écrit :
> This contradicts the documentation: (and would be rather rigid design)

Possible. But it has nothing to do with threading.

> > threaded decoder will just start the work and return EAGAIN.
> Thats true but this is not permitted by the text prior to my patch

Yes it is.

> The requiement to call avcodec_receive_frame() multiple times implies
> it does not return EAGAIN because you would not call it again if it
> did.

No, you would not call it again, you would first have to feed it another
packet. Still no problem at this level.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[Michael Niedermayer]

On Tue, Apr 18, 2017 at 03:23:14PM +0200, Nicolas George wrote:
> Le nonidi 29 germinal, an CCXXV, Michael Niedermayer a écrit :
> > if you cannot send a new packet before you received the output of
> > the previous
> 
> You have to TRY TO receive the output before you send a new packet. A

This contradicts the documentation: (and would be rather rigid design)

 * Using the API as outlined above is highly recommended. But it is also
 * possible to call functions outside of this rigid schema. For example, you can
 * call avcodec_send_packet() repeatedly without calling
 * avcodec_receive_frame(). In this case, avcodec_send_packet() will succeed
 * until the codec's internal buffer has been filled up (which is typically of
 * size 1 per output frame, after initial input), and then reject input with
 * AVERROR(EAGAIN). Once it starts rejecting input, you have no choice but to
 * read at least some output.


> threaded decoder will just start the work and return EAGAIN.

Thats true but this is not permitted by the text prior to my patch

The requiement to call avcodec_receive_frame() multiple times implies
it does not return EAGAIN because you would not call it again if it
did. (one might "mathematically" work around this but the result still
wont make sense)

For reference:

@@ -4960,7 +4960,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, 
AVSubtitle *sub,
  *  Unlike with older APIs, the packet is always fully 
consumed,
  *  and if it contains multiple frames (e.g. some audio 
codecs),
  *  will require you to call avcodec_receive_frame() multiple
- *  times afterwards before you can send a new packet.
+ *  times afterwards.



[...]


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[Nicolas George]

Le nonidi 29 germinal, an CCXXV, Michael Niedermayer a écrit :
> if you cannot send a new packet before you received the output of
> the previous

You have to TRY TO receive the output before you send a new packet. A
threaded decoder will just start the work and return EAGAIN.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[Michael Niedermayer]

On Tue, Apr 18, 2017 at 02:14:47PM +0200, wm4 wrote:
> On Tue, 18 Apr 2017 13:30:43 +0200
> Michael Niedermayer  wrote:
> 
> > On Tue, Apr 18, 2017 at 06:43:07AM -0400, Ronald S. Bultje wrote:
> > > Hi,
> > > 
> > > On Tue, Apr 18, 2017 at 6:31 AM, Michael Niedermayer 
> > >  > > > wrote:  
> > >   
> > > > Signed-off-by: Michael Niedermayer 
> > > > ---
> > > >  libavcodec/avcodec.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > index ee133712b5..2ac1523a36 100644
> > > > --- a/libavcodec/avcodec.h
> > > > +++ b/libavcodec/avcodec.h
> > > > @@ -4960,7 +4960,7 @@ int avcodec_decode_subtitle2(AVCodecContext 
> > > > *avctx,
> > > > AVSubtitle *sub,
> > > >   *  Unlike with older APIs, the packet is always fully
> > > > consumed,
> > > >   *  and if it contains multiple frames (e.g. some audio
> > > > codecs),
> > > >   *  will require you to call avcodec_receive_frame()
> > > > multiple
> > > > - *  times afterwards before you can send a new packet.
> > > > + *  times afterwards.  
> > > 
> > > 
> > > Does/did this imply single-threaded decoding?  
> > 
> > if you cannot send a new packet before you received the output of
> > the previous then the decoder has only one packet at a time and cannot
> > decode multiple in parallel.
> > 
> > ill add above to the commit message
> 
> That's not how it works. I'm against the patch.

Thats not how patch review works. If you have objections, you have to
explain the issue so it can be resolved/clarified.


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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[wm4]

On Tue, 18 Apr 2017 13:30:43 +0200
Michael Niedermayer  wrote:

> On Tue, Apr 18, 2017 at 06:43:07AM -0400, Ronald S. Bultje wrote:
> > Hi,
> > 
> > On Tue, Apr 18, 2017 at 6:31 AM, Michael Niedermayer 
> >  > > wrote:  
> >   
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/avcodec.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > index ee133712b5..2ac1523a36 100644
> > > --- a/libavcodec/avcodec.h
> > > +++ b/libavcodec/avcodec.h
> > > @@ -4960,7 +4960,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx,
> > > AVSubtitle *sub,
> > >   *  Unlike with older APIs, the packet is always fully
> > > consumed,
> > >   *  and if it contains multiple frames (e.g. some audio
> > > codecs),
> > >   *  will require you to call avcodec_receive_frame()
> > > multiple
> > > - *  times afterwards before you can send a new packet.
> > > + *  times afterwards.  
> > 
> > 
> > Does/did this imply single-threaded decoding?  
> 
> if you cannot send a new packet before you received the output of
> the previous then the decoder has only one packet at a time and cannot
> decode multiple in parallel.
> 
> ill add above to the commit message

That's not how it works. I'm against the patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[Michael Niedermayer]

On Tue, Apr 18, 2017 at 06:43:07AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Tue, Apr 18, 2017 at 6:31 AM, Michael Niedermayer  > wrote:
> 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/avcodec.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index ee133712b5..2ac1523a36 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -4960,7 +4960,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx,
> > AVSubtitle *sub,
> >   *  Unlike with older APIs, the packet is always fully
> > consumed,
> >   *  and if it contains multiple frames (e.g. some audio
> > codecs),
> >   *  will require you to call avcodec_receive_frame()
> > multiple
> > - *  times afterwards before you can send a new packet.
> > + *  times afterwards.
> 
> 
> Does/did this imply single-threaded decoding?

if you cannot send a new packet before you received the output of
the previous then the decoder has only one packet at a time and cannot
decode multiple in parallel.

ill add above to the commit message

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

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[wm4]

On Tue, 18 Apr 2017 12:31:25 +0200
Michael Niedermayer  wrote:

> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/avcodec.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index ee133712b5..2ac1523a36 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -4960,7 +4960,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, 
> AVSubtitle *sub,
>   *  Unlike with older APIs, the packet is always fully 
> consumed,
>   *  and if it contains multiple frames (e.g. some audio 
> codecs),
>   *  will require you to call avcodec_receive_frame() multiple
> - *  times afterwards before you can send a new packet.
> + *  times afterwards.
>   *  It can be NULL (or an AVPacket with data set to NULL and
>   *  size set to 0); in this case, it is considered a flush
>   *  packet, which signals the end of the stream. Sending the

How did the old text imply that, and how does the new text remove this
implication? Because I can't see either.

This removes the important fact that you can't just call receive at
"any time", but that it must happen before you call send again.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: Fix text implying single threaded decoding

[Ronald S. Bultje]

Hi,

On Tue, Apr 18, 2017 at 6:31 AM, Michael Niedermayer  wrote:

> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/avcodec.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index ee133712b5..2ac1523a36 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -4960,7 +4960,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx,
> AVSubtitle *sub,
>   *  Unlike with older APIs, the packet is always fully
> consumed,
>   *  and if it contains multiple frames (e.g. some audio
> codecs),
>   *  will require you to call avcodec_receive_frame()
> multiple
> - *  times afterwards before you can send a new packet.
> + *  times afterwards.


Does/did this imply single-threaded decoding?

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel