> On 1 Dec 2017, at 17:56, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
>>
>> On Tue, 2017-11-21 at 09:49 +0000, Frediano Ziglio wrote:
>>> This saves some resources if no client are connected.
>>> Also will allow to get a better capture engine taking into account
>>> client supported codecs.
>>>
>>> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
>>> ---
>>> src/spice-streaming-agent.cpp | 14 +++++++-------
>>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-
>>> agent.cpp
>>> index 23b9768..20f2925 100644
>>> --- a/src/spice-streaming-agent.cpp
>>> +++ b/src/spice-streaming-agent.cpp
>>> @@ -343,10 +343,6 @@ static void cursor_changes(Display *display, int
>>> event_base)
>>> static void
>>> do_capture(const char *streamport, FILE *f_log)
>>> {
>>> - std::unique_ptr<FrameCapture>
>>> capture(agent.GetBestFrameCapture());
>>> - if (!capture)
>>> - throw std::runtime_error("cannot find a suitable capture
>>> system");
>>> -
>>> streamfd = open(streamport, O_RDWR);
>>> if (streamfd < 0)
>>> // TODO was syslog(LOG_ERR, "Failed to open %s: %s\n",
>>> streamport, strerror(errno));
>>> @@ -361,9 +357,16 @@ do_capture(const char *streamport, FILE *f_log)
>>> }
>>> }
>>>
>>> + if (quit)
>>> + return;
>>> +
>>
>>
>> This hunk looks unrelated, no? Also, returning directly here bypasses
>> the 'done:' section at the end of the function. That doesn't seem
>> correct.
>>
>
> It is, previously the capture was build at the beginning so
> when quit was set code exited directly, now it won't.
>
>> (by the way, since we're using c++ here, couldn't we do the cleanup in
>> some sort of destructor rather than using a goto? )
>>
>
> In the current code there's no much difference, the device is closed
> anyway. There's another series (which currently seems stuck) that is
> providing the proper destructor.
Will try to unstuck that stuff this week.
>
>>
>>
>>> syslog(LOG_INFO, "streaming starts now\n");
>>> uint64_t time_last = 0;
>>>
>>> + std::unique_ptr<FrameCapture>
>>> capture(agent.GetBestFrameCapture());
>>> + if (!capture)
>>> + throw std::runtime_error("cannot find a suitable capture
>>> system”);
>>> +
Would it make sense (in a later patch) to have the error message generated
by GetBestFrameCapture, which might allow us to get a better sense of
why a specific plugin did not generate the capture?
>>> while (!quit && streaming_requested) {
>>> if (++frame_count % 100 == 0) {
>>> syslog(LOG_DEBUG, "SENT %d frames\n", frame_count);
>>> @@ -410,9 +413,6 @@ do_capture(const char *streamport, FILE *f_log)
>>> syslog(LOG_ERR, "FAILED to read command\n");
>>> goto done;
>>> }
>>> - if (!streaming_requested) {
>>> - capture->Reset();
>>> - }
>>> }
>>> }
>>>
>>
>>
>> Otherwise seems fine to me
>>
>> Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>>
Acked-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel