[vlc-devel] [PATCH] Add methods to libvlc to enable and query recording of current media:

[Mark Lee]

Hello...

On 29 July 2012 14:49, Rémi Denis-Courmont <remi at remlab.net> wrote:

> Le dimanche 29 juillet 2012 16:30:15 Mark Lee, vous avez écrit :
> > +/**
> > + * Can the media player record the current media?
> > + *
> > + * Media must be playing or buffering before it can be recorded.
> > + *
> > + * \param p_mi media player
> > + * \return non-zero if the media player can record, zero if it can not
> > + * \version LibVLC 2.1.0 or later
> > + */
> > +LIBVLC_API int libvlc_media_player_can_record( libvlc_media_player_t
> *p_mi
> > );
>
> I don't see how that's supposed to work. How does the caller know when to
> call
> the function? How does the caller know that the state has not changed
> since it
> checked the result of the function? This function seems intrinsically
> broken
> by design.
>

As to when: the caller could query this on receipt of a buffering or
playing event. Similarly if an event like stopped or media changed is
received then the caller would know the old state was no longer valid. Is
that approach not acceptable?

As to the state changing: I did not expect that the state of this variable
would change whilst the media was playing, but I suppose I don't know
enough about vlc to be sure.

Anyway, do you think it is simply useless to include this function or
should it be fixed somehow?


> > +
> > +/**
> > + * Set whether or not recording is enabled.
> > + *
> > + * Media must be buffering or playing before it can be recorded.
> > + *
> > + * Media will be saved to the appropriate media directory, for example
> > "~/Videos". The + * file will not be immediately available in that
> > directory until recording is finished. + *
> > + * Recording can be started and stopped on-the-fly once the media has
> > started playing, + * each time recording is stopped and restarted a new
> > file will be created. + *
> > + * Recording will be stopped when the media stops playing, and must be
> > explicitly enabled + * again to restart recording.
> > + *
> > + * Features such as next/previous chapter, set time or position and so
> on
> > are ineffective + * when recording is enabled.
> > + *
> > + * \param p_mi media player
> > + * \param i_record start recording if non-zero, stop recording if zero
> > + * \return zero on success, -1 on error
> > + * \version LibVLC 2.1.0 or later
> > + */
> > +LIBVLC_API int libvlc_media_player_set_record( libvlc_media_player_t
> > *p_mi, int i_record );
>
> You can use 'bool' here.
>

OK, I will change that.

I can submit a new patch, but I don't know what I can do about the prior
contentious can_record() method apart from removing it from the patch.

Maybe something similar to how is_seekable has been implemented is
necessary and possible, a new recordable_changed event?

Thanks for the comments.


> > +
> > +/**
> > + * Get whether or not the media is currently being recorded.
> > + *
> > + * \param p_mi media player
> > + * \return non-zero if recording, zero if not
> > + * \version LibVLC 2.1.0 or later
> > + */
> > +LIBVLC_API int libvlc_media_player_get_record( libvlc_media_player_t
> *p_mi
> > ); +
> >  /** @} media_player */
> >
> >  # ifdef __cplusplus
> > diff --git a/lib/libvlc.sym b/lib/libvlc.sym
> > index 974e04f..059471c 100644
> > --- a/lib/libvlc.sym
> > +++ b/lib/libvlc.sym
> > @@ -116,6 +116,7 @@ libvlc_media_new_from_input_item
> >  libvlc_media_parse
> >  libvlc_media_parse_async
> >  libvlc_media_player_can_pause
> > +libvlc_media_player_can_record
> >  libvlc_media_player_next_frame
> >  libvlc_media_player_event_manager
> >  libvlc_media_player_get_agl
> > @@ -129,6 +130,7 @@ libvlc_media_player_get_media
> >  libvlc_media_player_get_nsobject
> >  libvlc_media_player_get_position
> >  libvlc_media_player_get_rate
> > +libvlc_media_player_get_record
> >  libvlc_media_player_get_state
> >  libvlc_media_player_get_time
> >  libvlc_media_player_get_title
> > @@ -153,6 +155,7 @@ libvlc_media_player_set_media
> >  libvlc_media_player_set_nsobject
> >  libvlc_media_player_set_position
> >  libvlc_media_player_set_rate
> > +libvlc_media_player_set_record
> >  libvlc_media_player_set_time
> >  libvlc_media_player_set_title
> >  libvlc_media_player_set_xwindow
> > diff --git a/lib/media_player.c b/lib/media_player.c
> > index f30f184..4d93e42 100644
> > --- a/lib/media_player.c
> > +++ b/lib/media_player.c
> > @@ -1408,3 +1408,47 @@ void libvlc_media_player_next_frame(
> > libvlc_media_player_t *p_mi ) vlc_object_release( p_input_thread );
> >      }
> >  }
> > +
> > +int libvlc_media_player_can_record( libvlc_media_player_t *p_mi )
> > +{
> > +    input_thread_t *p_input_thread;
> > +    bool b_can_record;
> > +
> > +    p_input_thread = libvlc_get_input_thread( p_mi );
> > +    if( !p_input_thread )
> > +        return 0;
> > +
> > +    b_can_record = var_GetBool( p_input_thread, "can-record" );
> > +
> > +    vlc_object_release( p_input_thread );
> > +    return b_can_record;
> > +}
> > +
> > +int libvlc_media_player_set_record( libvlc_media_player_t *p_mi, int
> > i_record ) +{
> > +    input_thread_t *p_input_thread;
> > +
> > +    p_input_thread = libvlc_get_input_thread( p_mi );
> > +    if( !p_input_thread )
> > +        return -1;
> > +
> > +    var_SetBool( p_input_thread, "record", i_record );
> > +
> > +    vlc_object_release( p_input_thread );
> > +    return 0;
> > +}
> > +
> > +int libvlc_media_player_get_record( libvlc_media_player_t *p_mi )
> > +{
> > +    input_thread_t *p_input_thread;
> > +    bool b_record;
> > +
> > +    p_input_thread = libvlc_get_input_thread( p_mi );
> > +    if( !p_input_thread )
> > +        return 0;
> > +
> > +    b_record = var_GetBool( p_input_thread, "record" );
> > +
> > +    vlc_object_release( p_input_thread );
> > +    return b_record;
> > +}
>
>
> --
> Rémi Denis-Courmont
> http://www.remlab.net/
> http://fi.linkedin.com/in/remidenis
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120729/d1500770/attachment.html>