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

[Nicolas Adenis-Lamarre]

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.

Nicolas



2015-10-20 15:50 GMT+02:00 Michael Niedermayer <michael at niedermayer.cc>:

> On Sun, Oct 18, 2015 at 10:13:29PM +0200, Nicolas Adenis-Lamarre wrote:
> > The rtsp protocol requires the client to send a packet at the end of the
> > connexion.
> > FFmpeg basic network function check wether the user aborted the
> > communication and don't send the packet in this case.
> > So the protocol is not respected.
> > This commit removes the check. An other possibility would have to add an
> > extra parameter to these functions to force the packet sending.
>
> When the rt*p code and the connection is correctly functioning
> then ff_check_interrupt() will return 0
> When the code gets stuck due to the network connection or other
> side failing then ff_check_interrupt() can return non zero if the
> user application wants to get control back in a last ditch effort
> before killing the process or thread.
>
> Now i do no know enough to say what the problem is exactly
> maybe this is a bug in the user application and it simply should not
> use the interrupt callback to terminate things
> or maybe the application has a good reason why it needs to use this
> method, in which case maybe the API is too limited to indicate that
> clean termination is requested instead of imedeate abortion of all
> transfers.
> If this is due to limitations of the API then extending the API is
> welcome but it should be done in a way that doesnt break existing
> user applications
> I belive the patch here would break the case where a actual hard and
> interruption is wanted because maybe all network communication would
> timeout due to network being down or such
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> it is not once nor twice but times without number that the same ideas make
> their appearance in the world. -- Aristotle
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>