[gstreamer-bugs] [Bug 625944] New GstDiscoverer helper library

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=625944
  GStreamer | gst-plugins-base | git

--- Comment #11 from Edward Hervey <bilboed at gmail.com> 2010-08-05 14:30:23 UTC ---
(In reply to comment #10)
> Comments, part 1:
> 
> [looking at header file / API only for now]
> 
> - dependency on video/video.h for GstVideoFormat
>   is unfortunate. Is that info really needed? If
>   yes, for what / why / where?

   For converting the video caps information into end-user-friendly information
(i.e. all the GstVideoStreamInformation fields).

> 
> - GstStreamType / GstStream*Information: I think it
>   would be better if GstDiscoverer didn't use that
>   rather generic namespace, but instead namespaced
>   these things like GstDiscovererStream*. Makes
>   things longer, but more consistent IMHO. Could
>   also use nicer / more natural names then, like
>   GstDiscovererAudioStreamInfo.
> 
> - GstStream*Information: shorten Information to Info
>   everywhere? (Or just drop the Info completely?)

  How about the following:
  * GstDiscovererInfo
  * GstDiscovererStreamInfo (for the generic part)
  * GstDiscovererVideoInfo
  * GstDiscovererAudioInfo
  * GstDiscovererContainerInfo

  And then:
  * GST_DISCOVERER_STREAM_* for GstStreamType ?

> 
> - GstStreamType: no type (types?) for subtitle streams
>   (would want to differentiate text / bitmap somehow)

  Good point, I guess that could be easily addable
  * GST_DISCOVERER_STREAM_SUBTITLE
  * GstDiscovererSubtitleInfo
    * gboolean bitmap

> 
> - docs: "Audio media stream" => "Audio stream" etc.;
>   "successfull" -l ; "happend" => "happened"

  FIXED

> 
> - GstStream{Audio,Container,Video}Information:
>   I am not very much in favour of these structures
>   at all. I would make them completely opaque and
>   just provide accessor functions, e.g.:
> 
>     gst_discoverer_audio_stream_info_get_sample_rate(),
>     gst_discoverer_audio_stream_info_get_channels(), etc.
> 
>   I doubt there is a significant performance impact,
>   and it makes for much nicer API (IMHO) and is much more
>   bindings friendly.
> 
> - GstDiscovererInformation: same as above, make opaque
>   and provide accessors for everything
> 
> - GstStreamVideoInformation: GValues are weird here

  Sebastian made the same comment. I'll switch them to pairs of guint32.

> 
> - Does anyone ever need to go back to the previous info?

  I don't understand what you mean.

> 
> - We should wait for the TOC API to land before finalising
>   this API, so we can make sure it integrates well.

  I'd only care if you can show me one example of a URI with multiple chapters
that changes media type between chapters.

> 
> - GstDiscoverer: should be entirely opaque, with at most
>   a GstDiscovererPrivate * member in the structure
> 

  sure.

> 
> (There's more, but it's probably easier to discuss this in chunks)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.