[FFmpeg-devel] [PATCH] rtsp protocol : teardown packet not sent

[Michael Niedermayer]

On Tue, Oct 20, 2015 at 06:16:12PM +0200, Nicolas Adenis-Lamarre wrote:
> Let's take the example of ffplay in which the code always fails.
> ie : ffplay 'rtsp://
> mafreebox.freebox.fr/fbxtv_pub/stream?namespace=1&service=201&flavour=ld'
> fails to respect the protocol in 100% of the cases.
> 
> When i close the window,
> ffplay.c : stream_close() is called
> => is->abort_request = 1;
> => avformat_close_input(&is->ic);
> 
> then
> rstpdec.c : rtsp_read_close(AVFormatContext *s)
> => ff_rtsp_send_cmd_async(s, "TEARDOWN", rt->control_uri, NULL);
> => this call always fails because it calls sub routines like
> network.c:ff_network_wait_fd_timeout which fails if interrupt is done.
> 
> In my humble opinion, network.c:ff_network_wait_fd_timeout should not
> decicde to not send a packet because interrupt is done,
> it makes it impossible to send a packet after the interrup is done, which
> makes impossible to respect the rtsp protocol.
> In my humble opinion, the choice should be done in the upper stacks.
> 
> This is why i made this patch.
> An other suggestion if you fear to break something, is to add an extra
> parameter called "ignore_interrupt", to add a check, but in the absolute, i
> think the choice in my patch is preferable, even it could break things at
> the beginning, the time, people find where to add correctly the interrupt
> check at the good place, not in the low stack.


avio.h:
    /**
    * Callback for checking whether to abort blocking functions.
    * AVERROR_EXIT is returned in this case by the interrupted
    * function. During blocking operations, callback is called with
    * opaque as parameter. If the callback returns 1, the
    * blocking operation will be aborted.
    *
    * No members can be added to this struct without a major bump, if
    * new elements have been added after this struct in AVFormatContext
    * or AVIOContext.
    */
    typedef struct AVIOInterruptCB {

Thus if the callback is set for the protocol and if it returns 1
the fuction must abort if its (potentially) blocking. That also is
true for any teardown operations because they can block too.

you cannot just ignore the API as it is documented and disable
the interrupt callback machanism more or less entirely

What you can do is,
* extend/improve/fix the API
* change the user application to not return 1 from the callback
* not just copy the callback for the demuxer to the protocol used for
  tearing down the connection while still by some means ensuring that
  a user request for interrupting a possibly blocking demuxer is
  fullfilled
* There are probably other options, but the documented API must match
  the implementation and should be sane and also not do
  unexpected things to existing implementations writen based on the
  current API



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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151020/c6f949c9/attachment.sig>