[FFmpeg-devel] [PATCH] BluRay protocol using libbluray

[Michael Niedermayer]

On Mon, Feb 20, 2012 at 10:24:08AM +0200, Petri Hintukainen wrote:
> Michael Niedermayer wrote:
> > On Mon, Feb 13, 2012 at 01:22:02PM +0200, Petri Hintukainen wrote:
> > [...]
> > > +    BlurayContext *bd = (BlurayContext *)h->priv_data;
> > 
> > useless cast
> > 
> > 
> > > +    const BLURAY_DISC_INFO *disc_info;
> > > +
> > > +    disc_info = bd_get_disc_info(bd->bd);
> > > +    if (!disc_info) {
> > > +        av_log(h, AV_LOG_ERROR, "bd_get_disc_info() failed\n");
> > > +        return 0;
> > 
> > within ffmpeg negative values are generally used as errors
> > for consistency and avoiding confusing its probably best to do this
> > here too
> > 
> > 
> > [...]
> > > +static int bluray_open(URLContext *h, const char *path, int flags)
> > > +{
> > > +    BlurayContext *bd = (BlurayContext *)h->priv_data;
> > > +    int num_title_idx;
> > > +    const char *diskname = path;
> > > +
> > > +    av_strstart(path, BLURAY_PROTO_PREFIX, &diskname);
> > > +
> > > +    bd->bd = bd_open(diskname, NULL);
> > > +    if (!bd->bd) {
> > > +        av_log(h, AV_LOG_ERROR, "bd_open() failed\n");
> > > +        return AVERROR(EIO);
> > > +    }
> > > +
> > > +    /* check if disc can be played */
> > > +    if (!check_disc_info(h)) {
> > > +        return AVERROR(EIO);
> > > +    }
> > > +
> > > +    /* setup player registers */
> > > +    /* region code has no effect without menus
> > > +    if (bd->region > 0 && bd->region < 5) {
> > > +        av_log(h, AV_LOG_INFO, "setting region code to %d (%c)\n", bd->region, 'A' + (bd->region - 1));
> > > +        bd_set_player_setting(bd->bd, BLURAY_PLAYER_SETTING_REGION_CODE, bd->region);
> > > +    }
> > > +    */
> > > +
> > > +    /* load title list */
> > > +    num_title_idx = bd_get_titles(bd->bd, TITLES_RELEVANT, MIN_PLAYLIST_LENGTH);
> > 
> > > +    av_log(0, AV_LOG_INFO, "%d usable playlists:\n", num_title_idx);
> > 
> > s/0/h/
> > 
> > also, if you want to officially maintain this code for ffmpeg, then
> > please add yourself to MAINTAINERS as well
> > 
> > overall the code looks good
> 
> Updated patch attached.

applied


> 
> There's also lot of metadata available from libbluray. That could be
> used when ex. remuxing to matroska. Is there any way to set chapter
> info, title name, track languages, ... from protocol handler, or is the only way to
> export libbluray handle to mpeg-ts demuxer and fetch the data there ?

you could add a metadata field to URLProtocol but that wont be any
simpler

to maintain the bluray code, i suggest you get a public git repository
(for example github, gitorious, sf, ...)
you can just maintain the code there and once you are happy with
some changes, tell me and ill merge them into main ffmpeg.

Thanks

PS: one change that you probably should do is to rename bluray.c to
libbluray.c as this is more consistent to existing lib wrapers.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120303/e497524f/attachment.asc>