[FFmpeg-devel] [PATCH] avdevice: add fbdev output device

[Stefano Sabatini]

On date Tuesday 2013-10-22 23:27:46 +0200, Lukasz M encoded:
> On 22 October 2013 20:04, Stefano Sabatini <stefasab at gmail.com> wrote:
> 
> > > It is offtopic, I will probably come back with it later as RFC or
> > something.
> > > In general I mean that codec may support all formats but output device
> > only
> > > few of them.
> > > If device provide list of supported formats aside the codec formats list,
> > > then proper format
> > > may be chosen without user intaraction and without "guessing" basing on
> > > both list.
> > > I haven't analyse code so these are just simple thoughts, not sure it is
> > > applicable.
> >
> > Some devices support some combinations of formats/codecs (typically
> > rawvideo + a few supported formats), but I can think of output devices
> > accepting accepting encoded packets, thus you should probably consider
> > codec/format couples.
> >
> > I'm not sure if automatic choice can be fully automated (probably yes).
> >
> 
> OK, thanks for adive. I will come back to it in some time
> 
> 
> >
> > > +    if (ioctl(fbdev->fd, FBIOGET_VSCREENINFO, &fbdev->varinfo) < 0)
> > > +        av_log(h, AV_LOG_WARNING,
> > > +               "Error refreshing variable info: %s\n", strerror(errno));
> >
> > Note: strerror() is not thread-safe, please use av_str2err(AVERROR(errno))
> 
> 
> I missed that one. Fixed in attached patch.
> 
> 
> > > +
> > > +    pix_fmt = get_pixfmt_from_fb_varinfo(&fbdev->varinfo);
> > > +
> > > +    if (pix_fmt != fbdev->pix_fmt) {
> > > +        av_log(h, AV_LOG_ERROR, "Pixel format %s is not supported, use
> > %s\n",
> > > +               av_get_pix_fmt_name(fbdev->pix_fmt),
> > av_get_pix_fmt_name(pix_fmt));
> > > +        return AVERROR(EINVAL);
> > > +    }
> >
> > The logic and error message is a bit misleading. In case pixel format
> > changes dynamically it could be != fbdev->pix_fmt and still be a valid
> > pixel format. So you should probably first check in case you get
> > pix_fmt == NONE.
> >
> > Alternatively you fail if you notice that the pixel format changes.
> >
> 
> I think you misread this. fbdev->pix_fmt contained provided stream's pixel
> format.
> In case pix_fmt obtained from fb_varinfo is different than stream's format
> then conversion would be required.
> It doesn't matter pixel format obtained from fb_varinfo is supportable or
> not when it is different then stream's one.
> 
> I can't remember why I put video information in private context, but it is
> not required.
> I changed them to local variables and made variable names more descriptive.

> From c0c924e5b9216be40bfa7c99b4f790a012015a2b Mon Sep 17 00:00:00 2001
> From: Lukasz Marek <lukasz.m.luki at gmail.com>
> Date: Wed, 16 Oct 2013 17:18:55 +0200
> Subject: [PATCH] avdevice: add fbdev output device
> 
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  Changelog                |    1 +
>  configure                |    1 +
>  doc/outdevs.texi         |   29 ++++++
>  libavdevice/Makefile     |    1 +
>  libavdevice/alldevices.c |    2 +-
>  libavdevice/fbdev_enc.c  |  253 ++++++++++++++++++++++++++++++++++++++++++++++
>  libavdevice/version.h    |    2 +-
>  7 files changed, 287 insertions(+), 2 deletions(-)
>  create mode 100644 libavdevice/fbdev_enc.c

LGTM. I'll push it soon unless there are more comments from other
developers.

Thanks.
-- 
FFmpeg = Fantastic Fierce Magical Puristic Entertaining Goblin