[vlc-devel] Re: [PATCH] RTP PAUSE support..

[Brian]

It does set the stream in that state, it was actually the TEARDOWN  
code without the session removal, I agree it is dirty but it works  
correctly for the 10+ simultaneous clients I tested it with so I  
thought it might be good enough. I will see what I can do with the  
suggestion you made below.

- Brian


On Mar 27, 2007, at 3:26 AM, Jean-Paul Saman wrote:

> Brian wrote:
>> I noticed PAUSE wasn't implemented for RTSP sessions using the RTP  
>> module so I used the TEARDOWN code minus the client deletion and  
>> it seems to work correctly.
>
> I think there are problems with this code. It basically sets the  
> stream to the stop state and in that state it can be teared down.
>
>> Index: modules/stream_out/rtp.c
>> ===================================================================
>> --- modules/stream_out/rtp.c    (revision 19482)
>> +++ modules/stream_out/rtp.c    (working copy)
>> @@ -1698,8 +1698,42 @@
>>              break;
>>          }
>>          case HTTPD_MSG_PAUSE:
>> -            /* FIXME */
>> -            return VLC_EGENERIC;
>> +        {
>> +            rtsp_client_t *rtsp;
>> +
>> +            answer->i_status = 200;
>> +            answer->psz_status = strdup( "OK" );
>> +            answer->i_body = 0;
>> +            answer->p_body = NULL;
>> +
>> +            psz_session = httpd_MsgGet( query, "Session" );
>> +            rtsp = RtspClientGet( p_stream, psz_session );
>> +            if( rtsp && rtsp->b_playing )
>> +            {
>> +                int i_id;
>> +                /* FIXME */
>> +                rtsp->b_playing = VLC_FALSE;
> This is the dirt way of pausing. It messes with the state of the  
> input. You rather should be triggering the state itself.
>
> Something like this:
>
>     /* Toggle pause state */
>     i_state = var_GetInteger( p_instance->p_input, "state" );
>     if( i_state == PAUSE_S )
>         var_SetInteger( p_instance->p_input, "state", PLAYING_S );
>     else if( i_state == PLAYING_S )
>         var_SetInteger( p_instance->p_input, "state", PAUSE_S );
>
> And I probably forget some details too here.
>
>> +                vlc_mutex_lock( &p_sys->lock_es );
>> +                for( i_id = 0; i_id < rtsp->i_id; i_id++ )
>> +                {
>> +                    sout_stream_id_t *id = rtsp->id[i_id];
>> +                    int i;
>> +
>> +                    for( i = 0; i < p_sys->i_es; i++ )
>> +                    {
>> +                        if( id == p_sys->es[i] )
>> +                            break;
>> +                    }
>> +                    if( i >= p_sys->i_es ) continue;
>> +
>> +                    vlc_mutex_lock( &id->lock_rtsp );
>> +                    TAB_REMOVE( id->i_rtsp_access, id- 
>> >rtsp_access, rtsp->access[i_id] );
>> +                    vlc_mutex_unlock( &id->lock_rtsp );
>> +                }
>> +                vlc_mutex_unlock( &p_sys->lock_es );
>> +            }
>> +            break;
>> +        }
>>          case HTTPD_MSG_TEARDOWN:
>>          {
>>              rtsp_client_t *rtsp;
>> --This is the vlc-devel mailing-list, see http://www.videolan.org/ 
>> vlc/
>> To unsubscribe, please read http://developers.videolan.org/lists.html
>
> Please rewrite patch to do pausing in a clean way.
>
> Grtz,
> Jean-Paul Saman.
>
> -- 
> This is the vlc-devel mailing-list, see http://www.videolan.org/vlc/
> To unsubscribe, please read http://developers.videolan.org/lists.html
>

-- 
This is the vlc-devel mailing-list, see http://www.videolan.org/vlc/
To unsubscribe, please read http://developers.videolan.org/lists.html