[vlc-devel] [PATCH] lib: New API to use imem for access.
Rémi Denis-Courmont
remi at remlab.net
Fri Jan 17 15:35:15 CET 2014
Hello,
On Wed, 15 Jan 2014 15:53:55 +0000, Mark Lee
<mark.lee at capricasoftware.co.uk> wrote:
> diff --git a/include/vlc/libvlc_media_player.h
> b/include/vlc/libvlc_media_player.h
> index 27f7b97..37b3c2b 100644
> --- a/include/vlc/libvlc_media_player.h
> +++ b/include/vlc/libvlc_media_player.h
> @@ -635,6 +635,74 @@ LIBVLC_API
> void libvlc_audio_set_format( libvlc_media_player_t *mp, const char
> *format,
> unsigned rate, unsigned channels );
>
> +/**
> + * Callback prototype to open media for memory access (imem).
> + *
> + * \param data opaque pointer
> + * \param cookie text identifier
> + * \param location optional location for the media (from the parsed
imem
> MRL)
> + * \return zero on success, non-zero on error
> + */
> +typedef int (*libvlc_memory_open_cb)(void *data, const char *cookie,
> const char *location);
You can pass all the data you want in the location string, so I don't
really see the point for the cookie parameter in this case.
This fails to document what happens if there are multiple concurrent
instances of the same imem media. It should be possible for open to create
an opaque pointer for later callbacks of a specific instance.
> +/**
> + * Callback prototype to close media for memory access (imem).
> + *
> + * \param data opaque pointer
> + * \param cookie text identifier
> + */
> +typedef void (*libvlc_memory_close_cb)(void *data, const char *cookie);
Again, the cookie should be useless here.
> +
> +/**
> + * Callback prototype to read media data for memory access (imem).
> + *
> + * \param data opaque pointer
> + * \param cookie text identifier
> + * \param buffer_size size of the filled data buffer
> + * \param buffer pointer to the filled data buffer
> + * \return zero on success, non-zero on error (or EOF)
> + */
> +typedef int (*libvlc_memory_read_cb)(void *data, const char *cookie,
> size_t *buffer_size, void **buffer);
This fails to document the life cycle of the buffer and the input values
for size and pointer.
Also cookie is useless.
> +
> +/**
> + * Callback prototype to seek within media for memory access (imem).
> + *
> + * FIXME seeking is not currently supported by imem.
> + *
> + * \param data opaque pointer
> + * \param cookie text identifier
> + * \param offset offset within the buffer to seek to
> + * \return zero on success, non-zero on error
> + */
> +typedef int (*libvlc_memory_seek_cb)(void *data, const char *cookie,
> uint64_t offset);
Cookie is useless.
> +
> +/**
> + * Setup memory access callbacks (imem).
> + *
> + * This is used to provide 'raw' media data from an in-memory buffer
(e.g.
> + * reading a stream into a buffer).
> + *
> + * The 'data' and 'cookie' parameters can contain optional
> application-specific
> + * values.
> + *
> + * \param mp the media player
> + * \param open callback function to open the media, or NULL
> + * \param close callback function to close the media, or NULL
> + * \param read callback function to read data (must not be NULL)
> + * \param seek callback function to seek within the data, or NULL
> + * \param data opaque pointer
> + * \param cookie text identifier
> + * \return zero on success, non-zero on error
> + * \version LibVLC 2.2.0 or later
> + */
> +LIBVLC_API
> +int libvlc_memory_set_access_callbacks( libvlc_media_player_t *mp,
> + libvlc_memory_open_cb open,
> + libvlc_memory_close_cb close,
> + libvlc_memory_read_cb read,
> + libvlc_memory_seek_cb seek,
> + void *data, const char *cookie
);
IMHO, the function should create a media item, rather than modify a media
player instance. And then cookie is no longer necessary.
> +
> /** \bug This might go away ... to be replaced by a broader system */
>
> /**
> diff --git a/lib/libvlc.sym b/lib/libvlc.sym
> index 7c99668..edf560c 100644
> --- a/lib/libvlc.sym
> +++ b/lib/libvlc.sym
> @@ -186,6 +186,7 @@ libvlc_media_set_user_data
> libvlc_media_subitems
> libvlc_media_tracks_get
> libvlc_media_tracks_release
> +libvlc_memory_set_access_callbacks
> libvlc_new
> libvlc_playlist_play
> libvlc_release
> diff --git a/lib/media_player.c b/lib/media_player.c
> index c23dd42..5b975e5 100644
> --- a/lib/media_player.c
> +++ b/lib/media_player.c
> @@ -458,6 +458,15 @@ libvlc_media_player_new( libvlc_instance_t
*instance )
> var_Create (mp, "amem-rate", VLC_VAR_INTEGER | VLC_VAR_DOINHERIT);
> var_Create (mp, "amem-channels", VLC_VAR_INTEGER |
VLC_VAR_DOINHERIT);
>
> + /* Memory */
> + var_Create (mp, "imem-open", VLC_VAR_ADDRESS);
> + var_Create (mp, "imem-close", VLC_VAR_ADDRESS);
> + var_Create (mp, "imem-read", VLC_VAR_ADDRESS);
> + var_Create (mp, "imem-seek", VLC_VAR_ADDRESS);
> + var_Create (mp, "imem-data", VLC_VAR_STRING);
> + var_Create (mp, "imem-cookie", VLC_VAR_STRING);
> + var_Create (mp, "imem-cat", VLC_VAR_INTEGER);
> +
These should be set in an input item and then inherited by the input
thread, IMHO. I do not see a rationale for "polluting" the the media player
with these variables.
> /* Video Title */
> var_Create (mp, "video-title-show", VLC_VAR_BOOL);
> var_Create (mp, "video-title-position", VLC_VAR_INTEGER);
> @@ -981,6 +990,35 @@ void libvlc_audio_set_format( libvlc_media_player_t
> *mp, const char *format,
> var_SetInteger( mp, "amem-channels", channels );
> }
>
> +int libvlc_memory_set_access_callbacks( libvlc_media_player_t *mp,
> + libvlc_memory_open_cb open,
> + libvlc_memory_close_cb close,
> + libvlc_memory_read_cb read,
> + libvlc_memory_seek_cb seek,
> + void *data, const char *cookie)
> +{
> + if (!read)
> + return -1;
> +
> + var_SetAddress( mp, "imem-open", open );
> + var_SetAddress( mp, "imem-close", close );
> + var_SetAddress( mp, "imem-read", read );
> + var_SetAddress( mp, "imem-seek", seek );
> +
> + char *buf;
> + if ( asprintf( &buf, "%p", data ) == -1 )
> + return -1;
> +
> + var_SetString( mp, "imem-data", buf );
> + free( buf );
> +
> + var_SetString( mp, "imem-cookie", cookie );
> +
> + // Category 4 means "access"
> + var_SetInteger( mp, "imem-cat", 4 );
> +
> + return 0;
> +}
>
>
/**************************************************************************
> * Getters for stream information
> diff --git a/modules/access/imem.c b/modules/access/imem.c
> index 5b4b056..2a50521 100644
> --- a/modules/access/imem.c
> +++ b/modules/access/imem.c
> @@ -195,6 +195,11 @@ typedef int (*imem_get_t)(void *data, const char
> *cookie,
> size_t *, void **);
> typedef void (*imem_release_t)(void *data, const char *cookie, size_t,
> void *);
>
> +typedef int (*imem_open_t)(void *data, const char *cookie, const char
> *location);
> +typedef void (*imem_close_t)(void *data, const char *cookie);
> +typedef int (*imem_read_t)(void *data, const char *cookie, size_t
> *buffer_size, void **buffer);
> +typedef int (*imem_seek_t)(void *data, const char *cookie, uint64_t
> offset);
> +
>
/*****************************************************************************
> * Local prototypes
>
*****************************************************************************/
> @@ -211,6 +216,10 @@ typedef struct {
> struct {
> imem_get_t get;
> imem_release_t release;
> + imem_open_t open;
> + imem_close_t close;
> + imem_read_t read;
> + imem_seek_t seek;
> void *data;
> char *cookie;
> } source;
> @@ -246,20 +255,28 @@ static int OpenCommon(vlc_object_t *object,
> imem_sys_t **sys_ptr, const char *ps
> return VLC_ENOMEM;
>
> /* Read the user functions */
> - tmp = var_InheritString(object, "imem-get");
> - if (tmp)
> - sys->source.get = (imem_get_t)(intptr_t)strtoll(tmp, NULL, 0);
> - free(tmp);
> + sys->source.open = var_InheritAddress(object, "imem-open");
> + sys->source.close = var_InheritAddress(object, "imem-close");
> + sys->source.read = var_InheritAddress(object, "imem-read");
> + sys->source.seek = var_InheritAddress(object, "imem-seek");
>
> - tmp = var_InheritString(object, "imem-release");
> - if (tmp)
> - sys->source.release = (imem_release_t)(intptr_t)strtoll(tmp,
> NULL, 0);
> - free(tmp);
> -
> - if (!sys->source.get || !sys->source.release) {
> - msg_Err(object, "Invalid get/release function pointers");
> - free(sys);
> - return VLC_EGENERIC;
> + if (!sys->source.read)
> + {
> + tmp = var_InheritString(object, "imem-get");
> + if (tmp)
> + sys->source.get = (imem_get_t)(intptr_t)strtoll(tmp, NULL,
0);
> + free(tmp);
> +
> + tmp = var_InheritString(object, "imem-release");
> + if (tmp)
> + sys->source.release =
(imem_release_t)(intptr_t)strtoll(tmp,
> NULL, 0);
> + free(tmp);
> +
> + if (!sys->source.get || !sys->source.release) {
> + msg_Err(object, "Invalid get/release function pointers");
> + free(sys);
> + return VLC_EGENERIC;
> + }
> }
>
> tmp = var_InheritString(object, "imem-data");
> @@ -274,9 +291,15 @@ static int OpenCommon(vlc_object_t *object,
> imem_sys_t **sys_ptr, const char *ps
>
> sys->source.cookie = var_InheritString(object, "imem-cookie");
>
> - msg_Dbg(object, "Using get(%p), release(%p), data(%p), cookie(%s)",
> - sys->source.get, sys->source.release, sys->source.data,
> - sys->source.cookie ? sys->source.cookie : "(null)");
> + if (!sys->source.read)
> + msg_Dbg(object, "Using get(%p), release(%p), data(%p),
> cookie(%s)",
> + sys->source.get, sys->source.release, sys->source.data,
> + sys->source.cookie ? sys->source.cookie : "(null)");
> + else
> + msg_Dbg(object, "Using open(%p), close(%p), read(%p), seek(%p),
> data(%p), cookie(%s)",
> + sys->source.open, sys->source.close, sys->source.read,
> + sys->source.seek, sys->source.data,
> + sys->source.cookie ? sys->source.cookie : "(null)");
>
> /* */
> sys->dts = 0;
> @@ -310,6 +333,13 @@ static int OpenAccess(vlc_object_t *object)
> access->pf_seek = NULL;
> access->p_sys = (access_sys_t*)sys;
>
> + if (sys->source.open)
> + if (sys->source.open(sys->source.data, sys->source.cookie,
> access->psz_location))
> + {
> + CloseCommon(sys);
> + return VLC_EGENERIC;
> + }
> +
> return VLC_SUCCESS;
> }
>
> @@ -320,6 +350,10 @@ static void CloseAccess(vlc_object_t *object)
> {
> access_t *access = (access_t *)object;
>
> + imem_sys_t* sys = access->p_sys;
> + if (sys->source.close)
> + sys->source.close(sys->source.data, sys->source.cookie);
> +
> CloseCommon((imem_sys_t*)access->p_sys);
> }
>
> @@ -373,8 +407,17 @@ static block_t *Block(access_t *access)
> size_t buffer_size;
> void *buffer;
>
> - if (sys->source.get(sys->source.data, sys->source.cookie,
> - NULL, NULL, &flags, &buffer_size, &buffer)) {
> + imem_read_t read = sys->source.read;
> + if (read)
> + {
> + if (read(sys->source.data, sys->source.cookie, &buffer_size,
> &buffer))
> + {
> + access->info.b_eof = true;
> + return NULL;
> + }
> + }
> + else if (sys->source.get(sys->source.data, sys->source.cookie,
> + NULL, NULL, &flags, &buffer_size,
&buffer)) {
> access->info.b_eof = true;
> return NULL;
> }
> @@ -386,8 +429,9 @@ static block_t *Block(access_t *access)
> memcpy(block->p_buffer, buffer, buffer_size);
> }
>
> - sys->source.release(sys->source.data, sys->source.cookie,
> - buffer_size, buffer);
> + if (!read)
> + sys->source.release(sys->source.data, sys->source.cookie,
> + buffer_size, buffer);
> return block;
> }
--
Rémi Denis-Courmont
Sent from my collocated server
More information about the vlc-devel
mailing list