After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 797344 - gst-inspect: Pipe stdout to less if not piped already
gst-inspect: Pipe stdout to less if not piped already
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-10-27 14:53 UTC by Zeeshan Ali
Modified: 2018-10-28 13:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gst-inspect: Pipe stdout to less if not piped already (2.43 KB, patch)
2018-10-27 14:53 UTC, Zeeshan Ali
none Details | Review
DOESNT WORK: gst-inspect: Pipe stdout to less if not piped already (2.09 KB, patch)
2018-10-28 11:47 UTC, Zeeshan Ali
rejected Details | Review
gst-inspect: Pipe stdout to less if not piped already (3.20 KB, patch)
2018-10-28 12:57 UTC, Zeeshan Ali
committed Details | Review

Description Zeeshan Ali 2018-10-27 14:53:38 UTC
Just like all git commands do. Everyone I talked to at the GStreamer conference 2018, either liked or loved this idea so let's do it. :)
Comment 1 Zeeshan Ali 2018-10-27 14:53:42 UTC
Created attachment 374067 [details] [review]
gst-inspect: Pipe stdout to less if not piped already
Comment 2 Sebastian Dröge (slomo) 2018-10-27 15:04:58 UTC
I thought there's GLib API for doing this process stuff :)
Comment 3 Zeeshan Ali 2018-10-27 15:11:43 UTC
(In reply to Sebastian Dröge (slomo) from comment #2)
> I thought there's GLib API for doing this process stuff :)

I didn't find a glib API for isatty() at least.
Comment 4 Sebastian Dröge (slomo) 2018-10-27 15:17:33 UTC
No, but for spawning processes and redirecting stdout/etc
Comment 5 Zeeshan Ali 2018-10-27 15:20:21 UTC
(In reply to Sebastian Dröge (slomo) from comment #4)
> No, but for spawning processes and redirecting stdout/etc

I tried it but didn't manage to make it work. If this is going to be unix-specific anyway, i didn't see the point of spending time on making that work.
Comment 6 Sebastian Dröge (slomo) 2018-10-27 17:00:59 UTC
Review of attachment 374067 [details] [review]:

Generally looks good

::: tools/gst-inspect.c
@@ +1716,3 @@
+
+      while ((count = read (STDIN_FILENO, buffer, sizeof (buffer) - 1)) > 0)
+        write (STDOUT_FILENO, buffer, count);

Need to handle EAGAIN/EINTR here at least
Comment 7 George Kiagiadakis 2018-10-28 10:51:47 UTC
As an enhancement, I'd suggest to take a look at the PAGER env variable, which is generally used to set a viewer for things like man, for instance. git also respects it, try 'PAGER=more git log'
Comment 8 George Kiagiadakis 2018-10-28 10:58:23 UTC
(In reply to Zeeshan Ali from comment #5)
> (In reply to Sebastian Dröge (slomo) from comment #4)
> > No, but for spawning processes and redirecting stdout/etc
> 
> I tried it but didn't manage to make it work. If this is going to be
> unix-specific anyway, i didn't see the point of spending time on making that
> work.

It *can* work on non-unix too. Windows cmd always had partial tty support and nowdays (latest windows 10 builds) they have implemented full support even, so it can be made to work, but I think unix-only is a good start.
Comment 9 Zeeshan Ali 2018-10-28 11:47:09 UTC
Created attachment 374076 [details] [review]
DOESNT WORK: gst-inspect: Pipe stdout to less if not piped already

That's my attempt of using g_spawn_async_with_pipes instead. I also tried treating the returned child_stdin as the pipe itself and dup'ing the STDOUT fd over it (if that makes any sense) but that didn't work either. I think I'll have to manually create channels for both stdout and child_stdin and feed the data from former to the lattter.
Comment 10 Zeeshan Ali 2018-10-28 11:48:59 UTC
(In reply to Zeeshan Ali from comment #9)
> Created attachment 374076 [details] [review] [review]
> DOESNT WORK: gst-inspect: Pipe stdout to less if not piped already

Doesn't work == `less` never gets any data.
Comment 11 Zeeshan Ali 2018-10-28 11:53:04 UTC
Review of attachment 374067 [details] [review]:

::: tools/gst-inspect.c
@@ +1716,3 @@
+
+      while ((count = read (STDIN_FILENO, buffer, sizeof (buffer) - 1)) > 0)
+        write (STDOUT_FILENO, buffer, count);

for the write or read or both?
Comment 12 Sebastian Dröge (slomo) 2018-10-28 12:27:23 UTC
Comment on attachment 374076 [details] [review]
DOESNT WORK: gst-inspect: Pipe stdout to less if not piped already

Good with the other API, it's simple enough
Comment 13 Zeeshan Ali 2018-10-28 12:57:14 UTC
Created attachment 374078 [details] [review]
gst-inspect: Pipe stdout to less if not piped already

v2:

* Allow setting a custom pager with PAGER env.
* Handle EINTR and EAGAIN on read and write calls.
Comment 14 Sebastian Dröge (slomo) 2018-10-28 13:17:50 UTC
Review of attachment 374078 [details] [review]:

Go for it
Comment 15 Zeeshan Ali 2018-10-28 13:20:36 UTC
Attachment 374078 [details] pushed as 5d115a5 - gst-inspect: Pipe stdout to less if not piped already