[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [tyndur-devel] [PATCH] CDI/Sound
-------- Original-Nachricht --------
> Datum: Tue, 11 Aug 2009 20:08:32 +0200
> Von: Kevin Wolf <kevin@xxxxxxxxxx>
> An: tyndur-devel@xxxxxxxxxx
> Betreff: Re: [tyndur-devel] [PATCH] CDI/Sound
> On Tue, Aug 11, 2009 at 01:22:15AM +0200, Patrick@xxxxxxxxxxxxxxxxxxxx
> wrote:
> > From: deathly-sequences@xxxxxx <snake707@Cazad-Dum.(none)>
> >
> > ---
> > src/modules/cdi/ac97/Makefile.all | 6 +
> > src/modules/cdi/ac97/ac97.c | 189
> ++++++++++++++++++++++++++++++++++
> > src/modules/cdi/ac97/includes/ac97.h | 80 ++++++++++++++
> > src/modules/cdi/ac97/main.c | 123 ++++++++++++++++++++++
> > src/modules/cdi/include/cdi.h | 1 +
> > src/modules/cdi/include/cdi/sound.h | 151 +++++++++++++++++++++++++++
> > 6 files changed, 550 insertions(+), 0 deletions(-)
> > create mode 100644 src/modules/cdi/ac97/Makefile.all
> > create mode 100644 src/modules/cdi/ac97/ac97.c
> > create mode 100644 src/modules/cdi/ac97/includes/ac97.h
> > create mode 100644 src/modules/cdi/ac97/main.c
> > create mode 100644 src/modules/cdi/include/cdi/sound.h
>
> Ich ignoriere mal den AC97-Treiber, von dem hast du ja gesagt, dass er
> noch nicht fertig genug ist. Sag Bescheid, wenn ich ihn anschauen soll.
>
> > diff --git a/src/modules/cdi/include/cdi.h
> b/src/modules/cdi/include/cdi.h
> > index c6215a6..c3a48ee 100644
> > --- a/src/modules/cdi/include/cdi.h
> > +++ b/src/modules/cdi/include/cdi.h
> > @@ -25,6 +25,7 @@ typedef enum {
> > CDI_NETWORK = 1,
> > CDI_STORAGE = 2,
> > CDI_SCSI = 3,
> > + CDI_SOUND = 4,
> > } cdi_device_type_t;
> >
> > struct cdi_driver;
> > diff --git a/src/modules/cdi/include/cdi/sound.h
> b/src/modules/cdi/include/cdi/sound.h
> > new file mode 100644
> > index 0000000..680553a
> > --- /dev/null
> > +++ b/src/modules/cdi/include/cdi/sound.h
> > @@ -0,0 +1,151 @@
> > +
> > +/** @file sound.h
> > + *
> > + * Diese Datei enthält die Datenstrukturen, von denen die
> Datenstrukturen der
> > + * Treiber abgeleitet werden. Außerdem werden hier die
> Funktionsprototypen für
> > + * die Schnittstelle Soundtreiber, CDI definiert.
> > + **/
> > +
> > +/**
> > + * Treiber für die Soundkarten.
> > + *
> > + * @defgroup sound
> > + **/
> > +
> > +#ifndef __sound_h__
> > +#define __sound_h__
> > +
> > +#include <stdint.h>
> > +#include <stddef.h>
> > +
> > +#include "cdi.h"
> > +
> > +/* Angelehnt an net.h */
> > +
> > +struct cdi_sound_device{
> > + struct cdi_device dev;
> > + /* Im folgenden bezeichnet driver immer das Gerät. */
>
> Das klingt irgendwie auffällig falschrum. Ein Gerät sollte ein
> cdi_sound_device sein. Mit einem cdi_sound_driver kann der Treiber
> vermutlich nicht so viel anfangen, wenn er die ganzen Informationen
> ordentlich in einem cdi_sound_device abgelegt hat (wie das dein
> AC97-Treiber zu tun scheint)
gefixt.
>
> > + /* Ausführlichere Kommentare/Dokumentation findet man in den
> jeweiligen
> > + Implementationen der Treiber. Hier werden nur Funktionszeiger
> > + definiert. */
> > +
> > + /** @brief Damit das Gerät auch beginnt Daten abzuspielen.
> > + *
> > + * Weißt den Treiber an, Daten abzuspielen, wenn neue vorhanden
> sind.
> > + *
> > + * @param driver Treiber, der die Daten abspielen soll.
> > + *
> > + */
> > + void (*cdi_sound_play)(struct cdi_sound_driver *driver);
> > +
> > + /** @brief Hält die Audioausgabe des Treibers an.
> > + *
> > + * Wenn der Treiber nicht gerade von einer anderen Anwendung
> blockiert wird,
> > + * wird die Audioausgabe angehalten. Wird ignoriert, wenn der
> Treiber
> > + * 'in use' ist.
> > + *
> > + * @param driver Treiber, der die Audioausgabe anhalten soll.
> > + */
> > + void (*cdi_sound_stop)(struct cdi_sound_driver *driver);
>
> Ich überlege grad, ob das reicht. Irgendeine Lib dahinter wird ja
> vermutlich mehrere Sounds zusammenmixen. Wenn jetzt einer von zwei
> gerade abgespielten Sounds gestoppt wird, brauchen wir eigentlich keinen
> kompletten Stop, sondern...
>
> * ... ein Stop direkt gefolgt von einem Start, das den übriggebliebenen
> Sound wieder startet. Dazu brauchen wir wenigstens die aktuelle
> Position, an der angehalten wurde.
> * ... oder ein cdi_sound_replace() oder sowas, das einen neuen Puffer
> kriegt, dort die aktuelle Stelle raussucht und an die Soundkarte
> schickt, was jetzt zu spielen ist
>
> Ist der Gedanke so ungefähr nachvollziehbar?
>
Hmm ich glaube aber dass das unnötig ist. Da ein buffer weniger als eine Sekunde spielt. Wenn ein Sound gestoppt wird, kann ja CDI/Lib den übernächsten Buffer neuschreiben lassen. Bei 44,1 kHz abtastrate sind 2 buffer, maximal 1,49 Sekunden. Das heißt, wenn es einem wirklich auf speed ankommt kann man ab dem übernächsten Buffer alles neuschreiben. Der nächste macht wohl keinen sinn, da man nicht weiß wo die Ausführung im aktuelle Buffer gerade ist.
Es sollte jetzt auch nicht so häufig vorkommen, dass ein Sound so fix geändert werden muss. Wenn man kein quickmix reinbaut, dann ist es einfacher, weil man dann einfach anhalten kann, wenn der sound gerade gespielt wird und zum nächsten springen kann, oder den sound vorher einfach aus der queue löschen kann.
> +
>
> > + /** @brief Ein Prozess darf die in_use Option betätigen.
> > + *
> > + * Die in use Option ist mir eingefallen, als ich mir das
> Zusammenspiel
> > + * Amarok, Firefox mit flashplayer plugin und der rest des Systems
> angesehen
> > + * habe. Die schließen sich gegenseitig aus. Sobald Amarok an ist,
> kann
> > + * der Rest nichts mehr abspielen. Hat auch gute Seiten.
> Funktionszeiger ...
> > + *
> > + * @param driver Treiber der in use gestellt werden soll.
> > + * @param pid Prozess ID des Prozesses. Dient später nur zur
> Überprüfung.
> > + *
> > + * @return 0 bei Erfolg. 1 bei Misserfolg.
> > + *
> > + */
> > + uint32_t (*cdi_sound_in_use_enable)(struct cdi_sound_driver
> *driver,
> > + uint32_t pid);
> > +
>
> Zum einen solltest du drauf achten, keine Leerzeichen am Zeilenende zu
> haben (gibt noch einige mehr davon, hier ist es mir aufgefallen).
>
> Zum anderen ist das hier die falsche Ebene. Den Soundtreiber haben
> Sachen wie Prozesse nicht zu interessieren. Der Treiber spielt Sound ab,
> und das war's. Um Prozesse kann sich dann vielleicht die CDI-Lib
> kümmern.
gefixt.
>
> > + /** @brief Der gleiche Prozess darf das in_use Privileg abgeben.
> > + *
> > + * Anhand von pid wird nur überprüft ob der richtige Prozess die
> in use
> > + * Option abstellt. Funktionszeiger ...
> > + *
> > + * @param driver Treiber der die in use Option abschalten soll.
> > + * @param pid Prozessnummer.
> > + *
> > + * @return 0 bei Erfolg. 1 bei Misserfolg.
> > + */
> > + uint32_t (*cdi_sound_in_use_disable)(struct cdi_sound_driver
> *driver,
> > + uint32_t pid);
> > +
> > + /** @brief Funktionszeiger für eine Schnittstelle, in der man den
> aktuellen
> > + * Status des Treibers/Gerätes abrufen kann.
> > + *
> > + * Diese Funktion ist dafür gedacht um herauszufinden ob der
> Treiber gerade
> > + * Daten abspielt oder nicht. Hier ist die Schnittstelle
> Treiber/CDI
> > + *
> > + * @param driver Soundtreiber, dessen Zustand ermittelt werden
> soll.
> > + *
> > + * @return Die Returncodes können von Treiber zu Treiber
> variieren. Hier
> > + * ist eine Liste, von Codes, die auf jeden Fall genauso
> > + * implementiert werden sollen (Zusätzliche können
> angehangen
> > + * werden): 0 - Treiber/Gerät nicht
> arbeitsbereit/initialisiert.
> > + * 1 - Gerät arbeitsbereit. Spielt keine Daten ab. 2 -
> Treiber
> > + * erhält Daten. 4 - Gerät spielt Daten ab. Datenempfang
> möglich.
> > + * 8 - Gerät ist gestoppt, aber es sind noch Daten zum
> spielen
> > + * übrig. Neue Daten können jederzeit hinzugefügt
> werden.
> > + * 16 - Bit ist gesetzt, wenn Qucik Mixing (lineare
> Superposition
> > + * mit Clippingunterdrückung) erlaubt ist. 32 - Dieses Bit
> zeigt an
> > + * ob der Treiber gerade von einem Prozess "blockiert"
> wird.
> > + *
> > + */
> > + uint32_t (*cdi_sound_driver_state)(struct cdi_sound_driver
> *driver);
>
> Nein, sie kann nicht von Treiber zu Treiber variieren. Der Sinn und
> Zweck von CDI ist ein _standardisiertes_ Interface. Also: Ich schreibe
> anhand dieser Dokumentation eine CDI-Implementierung für mein OS, dann
> nehme ich einen beliebigen dazu passenden Soundtreiber und es
> funktioniert.
>
> Oder ich schreibe einen Soundtreiber, der sich daran hält, und kopiere
> ihn auf jedes beliebige OS, das CDI kann, und er funktioniert.
>
> An dieser Stelle braucht es also eine definitive Aussage, was es gibt
> und was nicht. Die Sachen, die es gibt, sollten als Konstante definiert
> werden (z.B. irgendwo ein enum für die zulässigen status) statt direkt
> magische Zahlen zu verwenden.
>
gefixt.
> > +
> > + /** @brief Funktionszeiger für eine Schnittstelle, die dem Treiber
> die
> > + * als nächstes abzuspielenden Daten übergibt.
> > + *
> > + * Dies ist ein Funktionszeiger und für die Schnittstelle Treiber
> CDI
> > + * gedacht.
> > + *
> > + * @param driver Soundtreiber an den die Daten geschickt werden
> sollen.
> > + * @param data_start Zeiger auf den Beginn der Daten. Es wird
> angenommen,
> > + * dass alle Daten hintereinander liegen.
> > + * @param data_length Länge des Datenblockes.
> > + * @param pid Datenbereiche werden Treiberintern mit einer PID
> versehen, um
> > + * diese z.B. noch zurückziehen zu können. Prozess
> beendet.
> > + * Datenbereiche wurden deallokiert.
> > + *
> > + * @return 0 - bei Erfolg, 1 - bei Fehler, wiederhohlung kann ihn
> beheben
> > + * 2 - Fehler nicht durch wiederhohlte Ausführung behebbar
> > + * kann. Wenn Gerät z.B. von einem Prozess benutzt wird,
> der Mixing
> > + * nicht zulassen will. Ansonnsten, werden Daten von
> verschiednen
> > + * Geräten linear superponiert, oder einfach nacheinander
> > + * abgespielt (Flags beachten).
> > + */
> > + uint32_t (*cdi_sound_input_data)(struct cdi_sound_driver *driver,
> > + uint32_t* data_start,
> > + uint32_t data_length,
> > + uint32_t pid);
>
> Siehe oben, pid geht den Treiber nichts an.
>
gefixt.
> Kevin
> _______________________________________________
> tyndur-devel mailing list
> tyndur-devel@xxxxxxxxxx
> http://list.tyndur.org/mailman/listinfo/tyndur-devel
--
GRATIS für alle GMX-Mitglieder: Die maxdome Movie-FLAT!
Jetzt freischalten unter http://portal.gmx.net/de/go/maxdome01