From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Dietmar Maurer <dietmar@proxmox.com>
Cc: "Marc-André Lureau" <marcandre.lureau@gmail.com>, qemu-devel@nongnu.org
Subject: Re: [PATCH v3 2/9] add vnc h264 encoder
Date: Thu, 24 Apr 2025 09:32:03 +0100 [thread overview]
Message-ID: <aAn3A6fMVu7paAOy@redhat.com> (raw)
In-Reply-To: <1814146671.4781.1745475565149@webmail.proxmox.com>
On Thu, Apr 24, 2025 at 08:19:25AM +0200, Dietmar Maurer wrote:
> > > > > + VNC_DEBUG("Could not add source to gst pipeline\n");
> > > > > + goto error;
> > > > > + }
> > > > > +
> > > > > + gst_object_ref(vs->h264->convert);
> > > > > + if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->convert)) {
> > > >
> > > > Can you use gst_bin_add_many() ?
> > >
> > > will try to use that.
>
> I really struggle to use those functions. Documentation states
> that gst_bin_add() can fail, but gst_bin_add_many() simply ignores
> the results of gst_bin_add() (explicitly stated in the docs)?
>
> Do you really want to use gst_bin_add_many() anyways?
Ignoring failure in gst_bin_add_many seems like a pretty dubious design
choice to me. Given they documented that as expected behaviour I guess
there's no chance getting gstreamer to fix that.
So I'd prefer we stick with what you've got so we handle errors correctly.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
next prev parent reply other threads:[~2025-04-24 8:33 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-18 11:29 [PATCH v3 0/9] Add VNC Open H.264 Encoding Dietmar Maurer
2025-04-18 11:29 ` [PATCH v3 1/9] new configure option to enable gstreamer Dietmar Maurer
2025-04-19 5:11 ` Marc-André Lureau
2025-04-23 12:14 ` Daniel P. Berrangé
2025-04-18 11:29 ` [PATCH v3 2/9] add vnc h264 encoder Dietmar Maurer
2025-04-19 5:24 ` Marc-André Lureau
2025-04-23 11:46 ` Dietmar Maurer
2025-04-23 11:57 ` Marc-André Lureau
2025-04-24 6:19 ` Dietmar Maurer
2025-04-24 8:32 ` Daniel P. Berrangé [this message]
2025-04-24 9:28 ` Dietmar Maurer
2025-04-24 9:34 ` Daniel P. Berrangé
2025-04-23 12:10 ` Daniel P. Berrangé
2025-04-23 12:25 ` Daniel P. Berrangé
2025-04-24 7:32 ` Dietmar Maurer
2025-04-24 8:43 ` Dietmar Maurer
2025-04-24 8:58 ` Daniel P. Berrangé
2025-04-24 10:39 ` Dietmar Maurer
2025-04-24 10:45 ` Daniel P. Berrangé
2025-04-24 11:01 ` Dietmar Maurer
2025-04-24 16:39 ` Daniel P. Berrangé
2025-04-18 11:29 ` [PATCH v3 3/9] vnc: h264: send additional frames after the display is clean Dietmar Maurer
2025-04-19 5:26 ` Marc-André Lureau
2025-04-23 12:39 ` Daniel P. Berrangé
2025-04-18 11:29 ` [PATCH v3 4/9] h264: search for available h264 encoder Dietmar Maurer
2025-04-23 12:43 ` Daniel P. Berrangé
2025-04-24 10:30 ` Daniel P. Berrangé
2025-04-18 11:29 ` [PATCH v3 5/9] h264: new vnc option to configure h264 at server side Dietmar Maurer
2025-04-21 10:06 ` Marc-André Lureau
2025-04-23 12:47 ` Daniel P. Berrangé
2025-04-25 8:02 ` Dietmar Maurer
2025-04-18 11:29 ` [PATCH v3 6/9] h264: add hardware encoders Dietmar Maurer
2025-04-23 12:49 ` Daniel P. Berrangé
2025-04-18 11:29 ` [PATCH v3 7/9] h264: do not reduce vnc update speed while we have an active h264 stream Dietmar Maurer
2025-04-23 12:50 ` Daniel P. Berrangé
2025-04-18 11:29 ` [PATCH v3 8/9] vnc: initialize gst during argument processing Dietmar Maurer
2025-04-21 10:09 ` Marc-André Lureau
2025-04-23 12:52 ` Daniel P. Berrangé
2025-04-18 11:29 ` [PATCH v3 9/9] h264: register shutdown notifiers, stop pipeline in destroy_encoder_context Dietmar Maurer
2025-04-21 10:14 ` Marc-André Lureau
2025-04-22 6:35 ` Dietmar Maurer
2025-04-22 6:39 ` Marc-André Lureau
2025-04-22 7:03 ` Dietmar Maurer
2025-04-22 7:07 ` Marc-André Lureau
2025-04-22 7:17 ` Dietmar Maurer
2025-04-23 12:58 ` Daniel P. Berrangé
2025-04-23 12:57 ` Daniel P. Berrangé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aAn3A6fMVu7paAOy@redhat.com \
--to=berrange@redhat.com \
--cc=dietmar@proxmox.com \
--cc=marcandre.lureau@gmail.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).