FFmpeg subprocess.Popen stdout leer?

[harryberlin]

wie muss es mit check_out() aussehen?
hatte heut etliches probiert, auch "check_out()" war dabei, aber ohne erfolg.
Wo entsteht und Was ist ein Zombie-Prozess?

[BlackJack]

@harryberlin: Ein Zombie-Prozess ist einer der zwar schon komplett durchgelaufen ist, wo aber der Elternprozess den Rückgabecode noch nicht abgefragt hat. Deshalb muss das Betriebssystem noch in der Prozesstabelle führen. Der ist also noch da, obwohl er eigentlich tot ist → Zombie. Die Wikipedia-Seite zu dem Begriff ist übrigens der erste Treffer wenn ich nach „zombie prozess“ suche…

[harryberlin]

also "shell=True" einfach weglassen, und dann passt es?
wie muss es mit check_out() aussehen?
hatte heut etliches probiert, auch "check_out()" war dabei, aber ohne erfolg.

[BlackJack]

@harryberlin: Naja nicht nur das `shell`-Argument weglassen sondern dann natürlich auch das Kommando als Liste angeben und nicht als Zeichenkette die erst von einer Shell interpretiert werden muss.

[harryberlin]

zu check_out:
wie muss es mit check_out() aussehen?
hatte heut etliches probiert, auch "check_out()" war dabei, aber ohne erfolg.

zu zombie:
also so?

Code: Alles auswählen

def run_command(command):
    import subprocess
    return subprocess.Popen(command.split(), stdout=subprocess.PIPE, stderr=subprocess.STDOUT).stdout.read()

[Sirius3]

@harryberlin: statt run_command rufst Du einfach subprocess.check_out(command) auf, wobei natürlich command eine Liste sein sollte und als solche schon erstellt wurde und nicht das command per split aufteilen, das ist erstens fehleranfällig und zweitens umständlich.

[harryberlin]

Habs jetzt umgeschrieben und bekomme einen error:

f = subprocess.check_output(UNMOUNT_DEVICE)
File "/usr/lib/python2.7/subprocess.py", line 573, in check_output
raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command '['sudo', 'umount', '/home/osmc/Android']' returned non-zero exit status 32

hier mein komplettes script:

Code: Alles auswählen

#!/usr/bin/env python
# -*- coding: utf-8 -*-

# sudo apt-get install jmtpfs

# sudo mkdir ~/Android
# sudo chmod 775 ~/Android
# sudo jmtpfs -o allow_other ~/Android

# sudo umount ~/Android
# sudo rmdir ~/Android


import os
import sys
import time
import xbmc
import xbmcgui
import xbmcaddon
import subprocess
from threading import Thread

__author__ = 'harryberlin'

DEVICE_NAMES = ['Android', 'userdefined']
ANDROID_PATH = os.path.expanduser("~") + "/Android"

APT_UPDATE = ['sudo', 'apt-get', 'update']
INSTALL_JMTPFS = ['sudo', 'apt-get', 'install', '-y', 'jmtpfs']
REMOVE_JMTPFS = ['sudo', 'apt-get', 'remove', '-y', 'jmtpfs']
MAKE_DIR = ['sudo', 'mkdir', '%s' % ANDROID_PATH]
REMOVE_DIR = ['sudo', 'rmdir', '%s' % ANDROID_PATH]
MOUNT_DEVICE = ['sudo', 'jmtpfs', '-o allow_other', '%s' % ANDROID_PATH]
UNMOUNT_DEVICE = ['sudo', 'umount', '%s' % ANDROID_PATH]


def ok(message1, message2=" ", message3=" "):
    xbmcgui.Dialog().ok(heading="MTP Connect", line1=message1, line2=message2, line3=message3)


def note(header, message=" ", time=3000):
    __icon__ = xbmcaddon.Addon().getAddonInfo('path') + '/icon.png'
    xbmcgui.Dialog().notification(heading=' %s' % header, message=' %s' % message, icon=__icon__, time=time)
    log('NOTIFICATION: ' + header + ' - ' + message)


def log(message):
    xbmc.log('plugin.script.mtpconnect: %s' % message)


def busy_show():
    xbmc.executebuiltin("ActivateWindow(busydialog)")


def busy_close():
    xbmc.executebuiltin("Dialog.Close(busydialog)")


def open_settings():
    xbmcaddon.Addon().openSettings()


def install():
    busy_show()
    log('update package libary')
    f = subprocess.check_output(APT_UPDATE)
    log(f)

    log('install package and add mount-path')

    f = subprocess.check_output(INSTALL_JMTPFS)
    log(f)

    f = subprocess.check_output(MAKE_DIR)
    log(f)
    busy_close()
    note("Package installed", "Path %s created" % ANDROID_PATH)


def deinstall():
    busy_show()
    log('delete directory')
    f = subprocess.check_output(REMOVE_JMTPFS)
    log(f)

    log('delete directory')
    f = subprocess.check_output(REMOVE_DIR)
    log(f)
    busy_close()
    note("Package deinstalled", "Path %s removed" % ANDROID_PATH)


def connect():
    disconnect(False)

    ok('Some Devices only share their Folders,', 'when Screen is unlocked.', 'UNLOCK YOUR DISPLAY')

    log('try to mount')
    f = subprocess.check_output(MOUNT_DEVICE)
    log(f)

    if f.find('No mtp devices found') > 0 or f.find('bad mount point') > 0 or f.find('jmtpfs: command not found') > 0:
        note("CAN'T CONNECT DEVICE")

    elif f.find('Device 0') > 0 or f.find('Device 1') > 0 or f.find('Device 2') > 0 or f.find('Device 3') > 0:
        note("Android Device connected", time=800)
        DIR_CHECKER.start()
    else:
        pass


def disconnect(shownotification=True):
    log('try to unmount')
    f = subprocess.check_output(UNMOUNT_DEVICE)
    log(f)
    if shownotification: note("Android Device disconnected")


def is_device_on():
    counter = 0
    while counter < 1:
        # log("check path %s" % counter)
        try:
            if os.listdir(ANDROID_PATH) == []:
                pass
            else:
                counter = -1
            time.sleep(1)
            counter += 1
        except:
            break
    disconnect()


def main():
    count = len(sys.argv) - 1
    if count > 0:
        given_args = sys.argv[1].split(';')
        if str(given_args[0]) == "install":
            install()
        elif str(given_args[0]) == "deinstall":
            deinstall()
        elif str(given_args[0]) == "connect":
            connect()
        elif str(given_args[0]) == "disconnect":
            disconnect(False)
        elif str(given_args[0]) == "settings":
            open_settings()
        else:
            note(header='Unknown Arguments given!', message='install, connect, disconnect are available', time=5000)

    else:
        open_settings()


DIR_CHECKER = Thread(name="dirchecker", target=is_device_on)
DIR_CHECKER.setDaemon(True)

if __name__ == '__main__':
    main()

[Sirius3]

@harryberlin: Du bekommst einen error, weil der umount-Befehl einen Fehler macht.

Zeile 26: expanduser kann auch mit ganzen Pfaden umgehen, ansonsten solltest Du os.path.join zum Zusammenfügen benutzen.
Zeile 31ff: die Stringformatierung ist jetzt ziemlich unnütz.
Zeile 66ff: die Variable f ist nicht nur schlecht benannt sondern auch überflüssig. Man könnte gleich log(check_out(..)) schreiben.
Zeile 102: hier kannst Du statt dem fehleranfälligen if die geworfene Exception benutzen. "No .." in f wäre hier auch besser als find.
Zeile 109: das else-pass ist überflüssig
Zeiel 139ff: das str ist überflüssig und die vielen ifs könnte man durch ein Wörterbuch ersetzen.

[harryberlin]

mir ist klar, dass man einiges besser coden kann. werd manches umsetzen.
ich habs mir schon fast gedacht, dass f ein dorn im auge ist. aber hierbei dürfte keine verwirrung entstehen.
was ist an if fehleranfällig?
und dennoch wird der fehler erzeugt, wie ich nicht weiß den zu lösen.
vorher lief es...

ist "check_out" das gleiche wie "check_output"?

[Sirius3]

@harryberlin: check_out ist ein Schreibfehler.
Die Fehlermeldungen können sich ändern, z.B. eine andere Sprache. ErrorCodes ändern sich nicht. Und was eine Exception ist, weißt Du?

[harryberlin]

ja, aber was bringt es mir wenn ich eine "try:" - "except:" draus mache? da kriege ich keine meldung zurück.

[Sirius3]

das machst Du draus:

Code: Alles auswählen

    try:
        log(subprocess.check_output(MOUNT_DEVICE))
    except subprocess.CalledProcessError:
        note("CAN'T CONNECT DEVICE")
    else:
        note("Android Device connected", time=800)

[BlackJack]

@harryberlin: Doch natürlich bekommst Du eine Meldung. Die Ausnahme an sich ist ja schon mal die Rückmeldung das etwas nicht geklappt hat. Und die Ausgabe des Programms bekommst Du auch, aber aus dem Gründen die Sirius3 schon genannt hat, solltest Du den Text nicht auswerten, sondern den Rückgabecode. Die Werte stehen in der ``umount`` Manpage nicht gelistet, aber es sind die gleichen Werte wie bei ``mount``.

[harryberlin]

@sirius3
darf ich in den raum werfen, irgendwie macht dies den code komplexer, weniger routinär und unflexibel.
wiederkehrende aufgaben sind jetzt eigenständiger code. und weiterhin, die error-meldung an sich wird nicht per log ausgegeben.

@blackjack
welche werte meinst du?
auf der manpage kann ich keine error values finden.
zudem bekomme ich diese eh nicht zurück.
weiterhin ist mount nur ein beispiel, wie man dem code entnehmen kann, sind andere befehle enthalten?

[BlackJack]

@harryberlin: Es macht den Code so komplex wie er sein muss, denn Deiner ist an den Stellen wo Du die Ausgabe nach Worten absuchst, schlicht falsch, weil das eben so nicht garantiert ist. Falls sich da Code wiederholt, kannst Du ja eine Funktion daraus machen die das ändert.

Was Du mit „routinär“ meinst, verstehe ich nicht, aber Fehler durch Ausnahmen anzuzeigen und zu behandeln ist auf jeden Fall idiomatischer als Fehlercodes oder gar potentiell veränderliche Ausgaben zu verarbeiten.

Ich meine die Ausname, die ja ein zusammengesetzter Wert ist. Da stecken alle Informationen drin die man braucht. Unter anderem auch der Rückgabecode des externen Prozesses, sollte er etwas anderes als 0 gewesen sein. Denn dann wird die Ausnahme ja ausgelöst.

Also bei mir ist die Manpage von ``mount`` sehr lang und gegen Ende kommt ein Abschnitt „RETURN CODES“.

Bei den anderen externen Befehlen muss man eben auch schauen was die an Rückgabewerten haben, wenn man gezielt auf irgendetwas davon eingehen will. Wobei es eventuell ja auch ausreicht überhaupt auf die Ausnahme an sich zu reagieren.

Diese Aufrufe könnte man auch erst einmal in eigene Funktionen auslagern, dann könnte man später überlegen die durch robusteren Code zu ersetzen. Zum Beispiel falls es bereits eine Bibliothek dafür gibt, oder dass man sich da selber mit, Beispielsweise mit dem `ctypes`-Modul Funktionen schreibt, welche die Systemaufrufe fürs (un)mounten direkt aufrufen, ohne einen Umweg über externe Programme.

[__deets__]

@harryberlin

Ergaenzend zu BlackJacks Anmerkungen: dein Vorgehen ist nicht robust gegenueber Aenderungen der Fehlertexte, sei es durch veraenderte Versionen (was natuerlich auch andere Probleme mit sich bringen kann), oder auch nur ein anderes locale, wodurch Fehlertexte in einer anderen Sprache ausgegeben werden koennen.

[BlackJack]

@harryberlin: Anmerkungen zum Code zusätzlich zu dem was Sirius3 schon gesagt hat:

Wenn man die Kommandozeilenargumente über ein Wörterbuch auf Funktionen abbildet, kann man die Meldung bei einem unbekannten Kommando leichter fehlerfrei hinbekommen, weil man die aus den Schlüsseln erstellen kann, statt das man immer daran denken muss bei Veränderungen bei den Kommandos auch diese Ausgabe entsprechend anzupassen.

`DEVICE_NAMES` wird nicht verwendet. `DIRCHECKER` ist keine Konstante und hat dort auf Modulebene auch nichts zu suchen. Dem Thread einen Namen zu geben, macht nicht wirklich Sinn.

Warum der Name `__icon__` in `note()`? Namen mit zwei führenden und folgenden Unterstrichen sind per Konvention für Werte vorgesehen die im Python-Ökosystem eine besondere Bedeutung haben, also entweder mit der Sprache oder mit Werkzeugen auf besondere Weise interagieren. Beispiele wären die ganzen ”magischen” Methoden die festlegen wie sich Objekte verhalten (Operatoren, ``for``, ``with``, …) oder `__doc__`, `__author__`, `__version__`, die von Hilfesystemen und Dokumentationsgeneratoren ausgewertet werden.

Da `busy_show()` und `busy_close()` immer in Paaren vorkommen, die Code ”umschliessen”, würde es sich anbieten da einen Contextmanager draus zu machen und ``with`` zu verwenden.

Wenn Du grundsätzlich die Ausgabe von externen Programmen protokollieren willst, dann bietet sich ja alleine deshalb schon eine Funktion dafür an. Damit man nicht jedes mal zusammen mit dem Aufruf noch einen `log()`-Aufruf schreiben muss.

Die `is_device_on()`-Funktion sieht sehr abenteuerlich aus. Erst einmal kann man den ``pass``-Zweig eleminieren. Dann sollte Ausnahmebehandlung präziser sein. Also weniger Code und nur die Ausnahme(n) umfassen, die man hier auch tatsächlich erwartet. Das ist also wohl das `os.listdir()` und eine `OSError`-Ausnahme, für den Fall das einem das Verzeichnis vor der Nase weggelöscht wird. Andererseits ist das `os.listdir()` in der Schleife die einzige Quelle für einen `OSError` und die Aktion ist das verlassen der Schleife. Also kann man die Ausnahmebehandlung auch um die Schleife legen, und damit den Sonderfall aus der Schleife herausziehen. Wenn man das gemacht hat, fällt auf, dass diese Ausnahmebehandlung dort nichts macht, also eigentlich nur dazu da ist das `disconnect()` davor zu schützen von einer Ausnahme übergangen zu werden. Also kann man ``try``/``finally`` verwenden.

Jetzt gilt es noch den gruseligen `counter` weg zu bekommen der nur Werte die Werte -1, 0, und 1 annehmen kann, aber eigentlich nur ein Wahrheitswert ist, der ungünstig ausgedrückt wurde. Denn eigentlich will man ja, dass die Schleife solange wartet, bis keine Dateien mehr in dem Pfad vorhanden sind. Das lässt sich *wesentlich* simpler ausdrücken.

Ich lande dann ungefähr hier (ungetestet):

Code: Alles auswählen

#!/usr/bin/env python
# -*- coding: utf-8 -*-
 
# sudo apt-get install jmtpfs
 
# sudo mkdir ~/Android
# sudo chmod 775 ~/Android
# sudo jmtpfs -o allow_other ~/Android
 
# sudo umount ~/Android
# sudo rmdir ~/Android

import os
import subprocess
import sys
import time
from contextlib import contextmanager
from threading import Thread

import xbmc
import xbmcaddon
import xbmcgui
 
__author__ = 'harryberlin'

ANDROID_PATH = os.path.expanduser('~/Android')
 
APT_UPDATE = ['sudo', 'apt-get', 'update']
INSTALL_JMTPFS = ['sudo', 'apt-get', 'install', '-y', 'jmtpfs']
REMOVE_JMTPFS = ['sudo', 'apt-get', 'remove', '-y', 'jmtpfs']
MAKE_DIR = ['sudo', 'mkdir', ANDROID_PATH]
REMOVE_DIR = ['sudo', 'rmdir', ANDROID_PATH]
MOUNT_DEVICE = ['sudo', 'jmtpfs', '-o', 'allow_other', ANDROID_PATH]
UNMOUNT_DEVICE = ['sudo', 'umount', ANDROID_PATH]
 
 
def log(message):
    xbmc.log('plugin.script.mtpconnect: %s' % message)


def notify(header, message=' ', display_duration=3000):
    xbmcgui.Dialog().notification(
        heading=header,
        message=message,
        icon=os.path.join(xbmcaddon.Addon().getAddonInfo('path') + 'icon.png'),
        time=display_duration,
    )
    log('NOTIFICATION: %s - %s' % (header, message))


@contextmanager
def busy_shown():
    xbmc.executebuiltin('ActivateWindow(busydialog)')
    try:
        yield
    finally:
        xbmc.executebuiltin('Dialog.Close(busydialog)')

 
def ask_confirmation(message1, message2=' ', message3=' '):
    return xbmcgui.Dialog().ok(
        heading='MTP Connect', line1=message1, line2=message2, line3=message3
    )


def open_settings():
    xbmcaddon.Addon().openSettings()


def call(command):
    try:
        try:
            output = subprocess.check_output(command, stderr=subprocess.STDOUT)
        except subprocess.CalledProcessError as error:
            output = error.output
            return error.returncode
        else:
            return 0
    finally:
        log(output)


def install():
    with busy_shown():
        log('update package libary')
        call(APT_UPDATE)
        log('install package and add mount-path')
        call(INSTALL_JMTPFS)
        call(MAKE_DIR)
    # 
    # FIXME This message is wrong if any of the calls above did not
    #   succeed.
    # 
    notify('Package installed', 'Path %s created' % ANDROID_PATH)
 
 
def deinstall():
    with busy_shown():
        log('remove package')
        call(REMOVE_JMTPFS)
        log('delete directory')
        call(REMOVE_DIR)
    # 
    # FIXME This message is wrong if any of the calls above did not
    #   succeed.
    # 
    notify('Package deinstalled', 'Path %s removed' % ANDROID_PATH)
 

def disconnect(show_notification=False):
    log('try to unmount')
    call(UNMOUNT_DEVICE)
    # 
    # FIXME This message may be wrong if any of the calls above did
    #   not succeed.
    # 
    if show_notification:
        notify('Android Device disconnected')

  
def wait_for_unmount():
    try:
        # 
        # FIXME Use a better indicator than an empty path.
        #   `os.path.ismount()` for instance.  Maybe even replace
        #   polling by `pyinotify` where available.
        # 
        while os.listdir(ANDROID_PATH):
            time.sleep(1)
    finally:
        disconnect(True)
 

def connect():
    disconnect()
    ask_confirmation(
        'Some Devices only share their Folders,',
        'when Screen is unlocked.',
        'UNLOCK YOUR DISPLAY'
    )
    log('try to mount')
    if call(MOUNT_DEVICE) == 0:
        notify('Android Device connected', 800)
        thread = Thread(target=wait_for_unmount)
        thread.daemon = True
        thread.start()
    else:
        notify("CAN'T CONNECT DEVICE")
 
 
def main():
    command_name2func = {
        'connect': connect,
        'deinstall': deinstall,
        'disconnect': disconnect,
        'install': install,
    }
    if len(sys.argv) > 1:
        command_name = sys.argv[1].partition(';')[0]
        command = command_name2func.get(command_name)
        if command:
            command()
        else:
            notify(
                'Unknown command given!',
                '{0} are available'.format(
                    ', '.join(sorted(command_name2func))
                ),
                5000
            )
    else:
        open_settings()


if __name__ == '__main__':
    main()

[harryberlin]

danke für die mühe. ich muss gestehen, ich bin etwas verärgert, der größte teil meines codes ist umgeschrieben. hat sozusagen den beigeschmack, dass alles mist war. sogar verständliche sachen hast du geändert. da wäre mir lieber gewesen, wenn wir mal kontakt gehabt hätten, um die gedankengänge auszutauschen, ggf. auch eine begründung zu nennen. da ich oft bei deinen umschreibenden erklärungen nicht mitkomme.
der code sind die anfänge und noch kein fertiges projekt, hätte ich vllt erwähnen sollen.

das wärs doch ungefähr gewesen. aber wozu das zweifache "try:"
log möchte ich evtl. gar nicht immer.

Code: Alles auswählen

def call(command):
    try:
        try:
            output = subprocess.check_output(command, stderr=subprocess.STDOUT)
        except subprocess.CalledProcessError as error:
            output = error.output
            return error.returncode
        else:
            return 0
    finally:
        log(output)

[BlackJack]

@harryberlin: Also in dem Code den Du gezeigt hattest, da wolltest Du immer `log()` aufrufen. Ich habe da halt nur das immer wiederkehrende Muster in eine Funktion herausgezogen.

Das zweite ``try`` ist da weil ich nicht genau wusste ob sich das ``finally`` auch auf den ``else``-Block beziehen würde. Tut es wahrscheinlich, aber so ist es auf jeden Fall eindeutig.

[Sirius3]

@BlackJack: Du wandelst eine Exception in einen Rückgabewert um???
Dann kann man sich das ganze ja gleich sparen:

Code: Alles auswählen

def call(command):
    process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
    output, _ = process.communicate()
    return_code = process.wait()
    log(output)
    return return_code