[FFmpeg-devel] Fwd: framebuffer device demuxer
Stefano Sabatini
stefano.sabatini-lala
Fri Jan 28 23:28:43 CET 2011
On date Friday 2011-01-28 16:32:38 +0000, M?ns Rullg?rd encoded:
> Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:
>
> > On date Friday 2011-01-28 01:10:46 +0000, M?ns Rullg?rd encoded:
> >
> >> I think "framebuffer" is a bit too generic a name. After all,
> >> anything with a display of any kind has a framebuffer. The Linux
> >> framebuffer device generally goes by the name "fbdev", so perhaps that
> >> would be a better name.
> >
> > Replaced with linuxfb, this should be more explicative of the fact
> > that it is a linux thing, this should prevent having users ask "I'm on
> > windows, why can't I find the fbdev device?".
>
> Works for me.
>
> >> > + if ((fb->fd = open(avctx->filename, flags)) == -1) {
> >> > + ret = AVERROR(errno);
> >> > + av_log(avctx, AV_LOG_ERROR, "Could not open framebuffer device '%s'.\n", avctx->filename);
> >> > + goto fail;
> >>
> >> You could return directly here and drop the check before close().
> >
> > Done.
>
> Not exactly...
>
> >> > + fb->width = fb_varinfo.xres;
> >> > + fb->height = fb_varinfo.yres;
> >> > + bits_per_pixel = fb_varinfo.bits_per_pixel;
> >> > +
> >> > + switch (bits_per_pixel) {
> >> > + case 8: pix_fmt = PIX_FMT_PAL8 ; break;
> >> > + case 15: pix_fmt = PIX_FMT_BGR555; break;
> >> > + case 16: pix_fmt = PIX_FMT_RGB565; break;
> >> > + case 24: pix_fmt = PIX_FMT_RGB24 ; break;
> >> > + case 32: pix_fmt = PIX_FMT_RGB32 ; break;
> >> > + default:
> >> > + ret = AVERROR(EINVAL);
> >> > + av_log(avctx, AV_LOG_ERROR, "Image with bits per pixel depth %d not supported.\n",
> >> > + bits_per_pixel);
> >> > + goto fail;
> >> > + }
> >>
> >> These are not the only formats possible. You cannot use bits per
> >> pixel exclusively to determine the format.
> >
> > Yes indeed. I'm using a far more robust solution now, yet incomplete,
> > also I have to see how to handle pal formats.
>
> There is also the issue of driver-specific formats, but let's ignore
> those for now.
I'm aware that the handling is not complete. I'll let to the other
developers to decide if this should block commit (as for me I'd prefer
to commit now and complete later).
> >> > +AVInputFormat framebuffer_demuxer = {
> >> > + .name = "framebuffer",
> >> > + .long_name = NULL_IF_CONFIG_SMALL("Linux framebuffer"),
> >> > + .priv_data_size = sizeof(FrameBufferContext),
> >> > + .read_header = framebuffer_read_header,
> >> > + .read_packet = framebuffer_read_packet,
> >> > + .read_close = framebuffer_read_close,
> >> > + .flags = AVFMT_NOFILE,
> >> > +};
> >>
> >> Is AVFMT_NOFILE really correct here? I'm not sure what its semantics
> >> are supposed to be.
> >
> > It's not explained in the docs, but AVFMT_NOFILE is used for all the
> > input and output device (possibly is what distinguishes a device from
> > a muxer/demuxer).
>
> OK, that's a reasonable explanation. Perhaps it should be documented
> somewhere.
>
> > And I'm still observing the 100% CPU problem noted by Luca A. in this
> > thread,
>
> That needs to be resolved, obviously.
>
> > a 0-memcpy solution is possible but it requires some change to the
> > framework (as the written packet contains aligned data, which is not
> > supported ATM).
>
> I don't think a zero-copy is a good idea here. The framebuffer is
> likely to be updated while encoding is in progress.
Yes.
> > @@ -2863,6 +2864,33 @@ fi
> >
> > texi2html -version > /dev/null 2>&1 && enable texi2html || disable texi2html
> >
> > +if enabled network; then
> > + check_type "sys/types.h sys/socket.h" socklen_t
> > + check_type netdb.h "struct addrinfo"
> > + check_type netinet/in.h "struct ipv6_mreq" -D_DARWIN_C_SOURCE
> > + check_type netinet/in.h "struct sockaddr_in6"
> > + check_type "sys/types.h sys/socket.h" "struct sockaddr_storage"
> > + check_struct "sys/types.h sys/socket.h" "struct sockaddr" sa_len
> > + # Prefer arpa/inet.h over winsock2
> > + if check_header arpa/inet.h ; then
> > + check_func closesocket
> > + elif check_header winsock2.h ; then
> > + check_func_headers winsock2.h closesocket -lws2 && \
> > + network_extralibs="-lws2" || \
> > + { check_func_headers winsock2.h closesocket -lws2_32 && \
> > + network_extralibs="-lws2_32"; }
> > + check_type ws2tcpip.h socklen_t
> > + check_type ws2tcpip.h "struct addrinfo"
> > + check_type ws2tcpip.h "struct ipv6_mreq"
> > + check_type ws2tcpip.h "struct sockaddr_in6"
> > + check_type ws2tcpip.h "struct sockaddr_storage"
> > + check_struct winsock2.h "struct sockaddr" sa_len
> > + else
> > + disable network
> > + fi
> > +fi
>
> Bad merge? This chunk was moved up a while ago.
Uh, yes after the last rebase.
[...]
> > +av_cold static int linuxfb_read_header(AVFormatContext *avctx, AVFormatParameters *ap)
> > +{
> > + FrameBufferContext *fb = avctx->priv_data;
> > + struct fb_var_screeninfo fb_varinfo;
> > + struct fb_fix_screeninfo fb_fixinfo;
> > + AVStream *st = NULL;
> > + enum PixelFormat pix_fmt;
> > + int ret, bytes_per_pixel, flags = O_RDONLY;
> > +
> > + if (!(st = av_new_stream(avctx, 0)))
> > + return AVERROR(ENOMEM);
> > +
> > + if (avctx->flags & AVFMT_FLAG_NONBLOCK)
> > + flags |= O_NONBLOCK;
> > +
> > + if ((fb->fd = open(avctx->filename, flags)) == -1) {
> > + ret = AVERROR(errno);
> > + av_log(avctx, AV_LOG_ERROR, "Could not open framebuffer device '%s': %s\n",
> > + avctx->filename, strerror(ret));
> > + close(fb->fd);
>
> Err... that should be "return ret", not close().
Doh!.. fixed.
[...]
> > + pix_fmt = get_pixfmt_from_fb_varinfo(fb_varinfo);
> > + if (pix_fmt == PIX_FMT_NONE) {
> > + ret = AVERROR(EINVAL);
> > + av_log(avctx, AV_LOG_ERROR, "Image with bits per pixel depth %d not supported.\n",
> > + fb_varinfo.bits_per_pixel);
>
> This error message doesn't quite match the error condition. I'd go
> with something like "Framebuffer pixel format not supported".
Fixed, I forgot to update after the last change.
> > + goto fail;
> > + }
> > +
> > + bytes_per_pixel = (fb_varinfo.bits_per_pixel + 7) >> 3;
> > + fb->frame_linesize = fb_varinfo.xres * bytes_per_pixel;
> > + fb->frame_size = fb->frame_linesize * fb_varinfo.yres;
> > + fb->linesize = fb_fixinfo.line_length;
> > + fb->time_base = ap->time_base;
> > + fb->time_frame = av_gettime() / av_q2d(ap->time_base);
>
> Don't initialise time_frame to current time. There could be any delay
> between init and first capture. Set it to something invalid instead,
> and check it in read_packet().
Good idea.
> > + fb->data = mmap(NULL, fb_fixinfo.line_length * (fb_varinfo.yres + fb_varinfo.yoffset),
> > + PROT_READ, MAP_SHARED, fb->fd, 0);
> > + fb->visible_data = fb->data +
> > + (fb_varinfo.xoffset + fb_varinfo.xres_virtual * fb_varinfo.yoffset) * bytes_per_pixel;
>
> This can change at any time if an application reconfigures the
> framebuffer. I'm not sure, but I think this also happens
> automatically when the text console scrolls.
Uhm OK, I start to understand the difference between fixinfo and
varinfo.
[...]
> > +static int linuxfb_read_packet(AVFormatContext *avctx, AVPacket *pkt)
> > +{
> > + FrameBufferContext *fb = avctx->priv_data;
> > + int64_t curtime, delay;
> > + struct timespec ts;
> > + int i, ret;
> > + uint8_t *pin, *pout;
> > +
> > + /* wait based on the frame rate */
> > + while (1) {
> > + curtime = av_gettime();
> > + delay = curtime - fb->time_frame * av_q2d(fb->time_base);
> > + if (delay >= 0) {
> > + fb->time_frame += INT64_C(1000000) * av_q2d(fb->time_base);
> > + break;
> > + }
> > + if (avctx->flags & AVFMT_FLAG_NONBLOCK)
> > + return AVERROR(EAGAIN);
> > + ts.tv_sec = delay / 1000000;
> > + ts.tv_nsec = (delay % 1000000) * 1000;
> > + nanosleep(&ts, NULL);
>
> This is asking for a negative delay. That's probably the cause of the
> CPU hogging.
There was a mess with the timestamps, now it should be fixed.
As for the CPU usage, it doesn't depend at all from the timestamps or
from the memcpy, simply seems that with big images ffplay/ffmpeg get
much slower:
[linuxfb @ 0xa40cdb0] w:1366 h:768 bpp:32 pixfmt:bgra tb:1/25 bit_rate:839270400
I replaced the framebuffer->pkt memcpy with a dummy->pkt memcpy
getting the same values.
Updated patch, not all the comments addressed (especially for the
varinfo/fixinfo stuff) so not yet ready for commit.
--
FFmpeg = Freak & Fantastic Monstrous Powered Elitarian God
More information about the ffmpeg-devel
mailing list