[Screeninvader] Goodbye Mr.Mplayer?

phaer hello at phaer.org
Thu Apr 9 18:41:56 CEST 2015


I don't have any reason against it, to the contrary, replacing it sounds
like a very good idea, now that there's the vdpau support that
ScreenInvader needs.

An additional argument for the replacement would be mpv's lua api[1]
which would probably be useful for advanced features, especially if we
could connect it to zeromq. If not, JSON-based IPC[2] looks good enough

[1]: https://github.com/mpv-player/mpv/blob/master/DOCS/man/lua.rst
[2]: https://github.com/mpv-player/mpv/blob/master/DOCS/man/ipc.rst

Amir Hassan <amir at viel-zu.org> writes:

> Mplayer has been a (/the most?) vital component for the ScreenInvader from
> the beginning, and as such the quality requirements have been pretty high
> - and largely met. Anyway. during the ongoing rewrite of the ScreenInvader
> several issue have been encountered they aren't easy to workaround:
>
> 1. missing https support for mplayer requires us to run proxy.py and
> maintain a list of https only services.
> 2. slave command "run" doesn't reap forks and generates zombie processes
> 3. tracking player events and reading mplayer properties requires parsing
> stdout
>
> I went through our new code base and looked out for the places where there
> is still real ugly code and guess what?
>
> Anyway, I'm an early mplayer user ever since and so i reached out for help
> in the mplayer community.
>
> first i tried to tackle the https issue and found following post with an
> attempt to submit patches for https support:
> http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2007-February/049277.html
> In short - the patch was rejected because it _uses_ fork ("... because
> it's ugly, creates a mess, prevents certain things from working..."
>
> then i tracked down the "run" bug. i quickly found the pertaining code
> section in commands.c
>
> 3370        case MP_CMD_RUN:
> 3371	#ifndef __MINGW32__
> 3372	            if (!fork()) {
> 3373	                char *exp_cmd = property_expand_string(mpctx,
> cmd->args[0].v.s);
> 3374	                if (exp_cmd) {
> 3375	                    execl("/bin/sh", "sh", "-c", exp_cmd, NULL);
> 3376	                    free(exp_cmd);
> 3377	                }
> 3378	                exit(0);
> 3379	            }
> 3380	#endif
> 3381	            break;
>
> the interesting part is it does _use_ fork. and it does it in a very bad
> way - without waiting for the process to terminate and reaping its return
> value. doing that generates zombie processes, which are usually no harm
> unless e.g you generate lots of them on an embedded system or you run
> mplayer as a daemon. we do both because we're are trying to workaround 3
> by making mplayer print the time_pos and length to stdout every second.
> that's a zombie per second that doesn't get reaped until mplayer exits
> (which by design never should happen :p).
> well, bugs happen and so i'm right now waiting for an answer to
> https://lists.mplayerhq.hu/pipermail/mplayer-users/2015-April/087953.html.
> _but_ while waiting for an answer i answered a request for help myself
> (https://lists.mplayerhq.hu/pipermail/mplayer-users/2015-April/087956.html)
> and in short - advised to use MPV :D
>
> So... there have been two reasons i haven't already switched our codebase
> to MPV:
> 1. It would require a rewrite of the ScreenInvader
> 2. MPV's VDPAU acceleration doesn't work with libvdpau-sunxi
>
> Both reasons are the past since we're rewriting right now and
> libvdpau-sunxi _does_ now support MPV:
> https://github.com/linux-sunxi/libvdpau-sunxi/blob/master/README
>
> Can you come up with a real good reason why we shouldn't switch?
>
> greets,
> amir
>
> _______________________________________________
> Screeninvader mailing list
> Screeninvader at lists.metalab.at
> https://lists.metalab.at/mailman/listinfo/screeninvader

--
Sent with my mu4e



More information about the Screeninvader mailing list