[Merge] lp:~morphis/aethercast/hw-encoding-support into lp:aethercast

Konrad Zapałowicz konrad.zapalowicz at canonical.com
Fri Feb 26 17:15:56 UTC 2016


Managed to read half of the change. Just minor comments mostly related to signed/unsigned usage.

Diff comments:

> 
> === added file 'src/mcs/mir/streamconnector.cpp'
> --- src/mcs/mir/streamconnector.cpp	1970-01-01 00:00:00 +0000
> +++ src/mcs/mir/streamconnector.cpp	2016-02-25 13:26:42 +0000
> @@ -0,0 +1,204 @@
> +/*
> + * Copyright (C) 2015 Canonical, Ltd.
> + *
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 3, as published
> + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranties of
> + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
> + * PURPOSE.  See the GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include <boost/concept_check.hpp>
> +
> +#include "mcs/logger.h"
> +#include "mcs/mir/streamconnector.h"
> +
> +namespace {
> +static constexpr const char *kMirSocket{"/run/mir_socket"};
> +static constexpr const char *kMirConnectionName{"aethercast screencast client"};
> +}
> +
> +namespace mcs {
> +namespace mir {
> +
> +std::string StreamConnector::DisplayModeToString(const DisplayMode &mode) {
> +    switch (mode) {
> +    case DisplayMode::kExtend:
> +        return "extend";
> +    case DisplayMode::kMirror:
> +        return "mirror";
> +    default:
> +        break;
> +    }
> +    return "unknown";
> +}
> +
> +StreamConnector::Ptr StreamConnector::Create(const StreamConnector::DisplayOutput &output) {
> +    return std::shared_ptr<StreamConnector>(new StreamConnector(output));
> +}
> +
> +StreamConnector::StreamConnector(const StreamConnector::DisplayOutput &output) :
> +    output_(output) {
> +    connection_ = mir_connect_sync(kMirSocket, kMirConnectionName);
> +    if (!mir_connection_is_valid(connection_)) {
> +        MCS_ERROR("Failed to connect to Mir server: %s",
> +                  mir_connection_get_error_message(connection_));
> +        return;
> +    }
> +
> +    auto config = mir_connection_create_display_config(connection_);
> +
> +    MirDisplayOutput *active_output = nullptr;
> +    int output_index = 0;

Could be unsigned

> +
> +    for (unsigned int i = 0; i < config->num_outputs; ++i) {
> +        if (config->outputs[i].connected &&
> +            config->outputs[i].used &&
> +            config->outputs[i].current_mode < config->outputs[i].num_modes) {
> +            // Found an active connection we can just use for our purpose
> +            active_output = &config->outputs[i];
> +            output_index = i;
> +            break;
> +        }
> +    }
> +
> +    if (!active_output) {
> +        MCS_ERROR("Failed to find a suitable display output");
> +        return;
> +    }
> +
> +    const MirDisplayMode *display_mode = &active_output->modes[active_output->current_mode];
> +
> +    params_.height = display_mode->vertical_resolution;
> +    params_.width = display_mode->horizontal_resolution;
> +
> +    if (output_.mode == DisplayMode::kMirror) {
> +        params_.region.left = 0;
> +        params_.region.top = 0;
> +        params_.region.width = params_.width;
> +        params_.region.height = params_.height;
> +
> +        output_.width = params_.width;
> +        output_.height = params_.height;
> +    }
> +    else if (output_.mode == DisplayMode::kExtend) {
> +        // If we request a screen region outside the available screen area
> +        // mir will create a mir output which is then available for everyone
> +        // as just another display.
> +        params_.region.left = params_.width;
> +        params_.region.top = 0;
> +        params_.region.width = output_.width;
> +        params_.region.height = output_.height;
> +
> +        params_.width = output_.width;
> +        params_.height = output_.height;
> +    }
> +
> +    output_.refresh_rate = display_mode->refresh_rate;
> +
> +    MCS_INFO("Selected output ID %i [(%ix%i)+(%ix%i)] orientation %d",
> +             output_index,
> +             params_.width, params_.height,
> +             params_.region.left, params_.region.top,
> +             active_output->orientation);
> +
> +    MCS_DEBUG("Setting up screencast [%s %dx%d]",
> +              DisplayModeToString(output_.mode),
> +              output_.width,
> +              output_.height);
> +
> +    unsigned int num_pixel_formats = 0;
> +    mir_connection_get_available_surface_formats(connection_, &params_.pixel_format,
> +                                                 1, &num_pixel_formats);
> +    if (num_pixel_formats == 0) {
> +        MCS_ERROR("Failed to find suitable pixel format: %s",
> +                  mir_connection_get_error_message(connection_));
> +        return;
> +    }
> +
> +    screencast_ = mir_connection_create_screencast_sync(connection_, &params_);
> +    if (!screencast_) {
> +        MCS_ERROR("Failed to create Mir screencast: %s",
> +                  mir_connection_get_error_message(connection_));
> +        return;
> +    }
> +
> +    buffer_stream_ = mir_screencast_get_buffer_stream(screencast_);
> +    if (!buffer_stream_) {
> +        MCS_ERROR("Failed to setup Mir buffer stream");
> +        return;
> +    }
> +
> +    auto platform_type = mir_buffer_stream_get_platform_type(buffer_stream_);
> +    if (platform_type != mir_platform_type_android) {
> +        MCS_ERROR("Not running with android platform: This is not supported.");
> +        mir_buffer_stream_release_sync(buffer_stream_);
> +        buffer_stream_ = nullptr;
> +        return;
> +    }
> +}
> +
> +StreamConnector::~StreamConnector() {
> +    if (screencast_)
> +        mir_screencast_release_sync(screencast_);
> +
> +    if (connection_)
> +        mir_connection_release(connection_);
> +}
> +
> +void StreamConnector::SwapBuffersSync() {
> +    if (!buffer_stream_)
> +        return;
> +
> +    mir_buffer_stream_swap_buffers_sync(buffer_stream_);
> +}
> +
> +void StreamConnector::SwapBuffers() {
> +    if (!buffer_stream_)
> +        return;
> +
> +    mir_buffer_stream_swap_buffers(buffer_stream_, [](MirBufferStream *stream, void *client_context) {
> +        boost::ignore_unused_variable_warning(stream);
> +        boost::ignore_unused_variable_warning(client_context);
> +
> +        MCS_DEBUG("Buffers are swapped now");
> +
> +    }, nullptr);
> +}
> +
> +bool StreamConnector::IsValid() const {
> +    return connection_ && screencast_ &&  buffer_stream_;
> +}
> +
> +void* StreamConnector::NativeWindowHandle() const {
> +    if (!buffer_stream_)
> +        return nullptr;
> +
> +    return reinterpret_cast<void*>(mir_buffer_stream_get_egl_native_window(buffer_stream_));
> +}
> +
> +void* StreamConnector::NativeDisplayHandle() const {
> +    if (!connection_)
> +        return nullptr;
> +
> +    return mir_connection_get_egl_native_display(connection_);
> +}
> +
> +StreamConnector::DisplayOutput StreamConnector::OutputMode() const {
> +    return output_;
> +}
> +
> +MirNativeBuffer* StreamConnector::CurrentBuffer() const {
> +    MirNativeBuffer *buffer = nullptr;
> +    mir_buffer_stream_get_current_buffer(buffer_stream_, &buffer);
> +    return buffer;
> +}
> +} // namespace mir
> +} // namespace mcs
> 
> === modified file 'src/mcs/miracastservice.cpp'
> --- src/mcs/miracastservice.cpp	2016-01-21 13:25:31 +0000
> +++ src/mcs/miracastservice.cpp	2016-02-25 13:26:42 +0000
> @@ -122,6 +130,9 @@
>              g_unix_signal_add(SIGINT, OnSignalRaised, this);
>              g_unix_signal_add(SIGTERM, OnSignalRaised, this);
>  
> +            // Initialize gstreamer.
> +            mcs::InitGstreamerOnceOrThrow();

Do not see the wider scope but is there a try, catch somewhere?

> +
>              // Redirect all wds logging to our own.
>              wds::LogSystem::set_vlog_func(SafeLog<mcs::Logger::Severity::kTrace>);
>              wds::LogSystem::set_log_func(SafeLog<mcs::Logger::Severity::kInfo>);
> 
> === modified file 'src/mcs/miracastsourceclient.cpp'
> --- src/mcs/miracastsourceclient.cpp	2015-12-09 16:07:13 +0000
> +++ src/mcs/miracastsourceclient.cpp	2016-02-25 13:26:42 +0000
> @@ -82,7 +83,15 @@
>  }
>  
>  std::string MiracastSourceClient::GetLocalIPAddress() const {
> -    return local_address_;
> +    return local_address_.to_string();
> +}
> +
> +int MiracastSourceClient::GetNextCSeq(int *initial_peer_cseq) const {
> +    static int send_cseq = 0;

Could that be declared unsigned? If so then the function should return unsigned too.

> +    ++send_cseq;
> +    if (initial_peer_cseq && send_cseq == *initial_peer_cseq)
> +        send_cseq *= 2;
> +    return send_cseq;
>  }
>  
>  class TimerCallbackData {
> 
> === modified file 'src/mcs/networkutils.cpp'
> --- src/mcs/networkutils.cpp	2016-01-15 19:31:22 +0000
> +++ src/mcs/networkutils.cpp	2016-02-25 13:26:42 +0000
> @@ -259,4 +274,47 @@
>          available = (int64_t) nbytes;
>      return available;
>  }
> +
> +typedef struct {
> +#ifdef SUPPORT_64BIT
> +    u64 bufaddr;
> +#else
> +    char *bufaddr;
> +#endif
> +    int used_len;

This is used with conjunction with size_t thus, unless there are reasons, should be declared unsigned.

> +    int total_len;

This is used with conjunction with size_t this, unless there are reasons, should be declared unsigned.

> +} android_wifi_priv_cmd;
> +
> +int NetworkUtils::SendDriverPrivateCommand(const std::string &ifname, const std::string &cmd) {
> +    struct ifreq ifr;
> +    int ret = 0, s;
> +    android_wifi_priv_cmd priv_cmd;
> +    char buf[kDriverCommandReplySize];
> +    size_t buf_len = kDriverCommandReplySize;
> +
> +    ::memset(buf, 0, sizeof(buf));
> +    ::memcpy(buf, cmd.c_str(), cmd.length() + 1);
> +    ::memset(&ifr, 0, sizeof(ifr));
> +    ::memset(&priv_cmd, 0, sizeof(priv_cmd));
> +
> +    ::strncpy(ifr.ifr_name, ifname.c_str(), IFNAMSIZ);
> +
> +#ifdef SUPPORT_64BIT
> +    priv_cmd.bufaddr = (u64)(uintptr_t) buf;
> +#else
> +    priv_cmd.bufaddr = buf;
> +#endif
> +    priv_cmd.used_len = buf_len;
> +    priv_cmd.total_len = buf_len;
> +    ifr.ifr_data = &priv_cmd;
> +
> +    s = ::socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
> +    if (s < 0)
> +        return -EIO;
> +
> +    ret = ::ioctl(s, SIOCDEVPRIVATE + 1, &ifr);
> +    ::close(s);
> +    return ret;
> +}
> +
>  } // namespace mcs


-- 
https://code.launchpad.net/~morphis/aethercast/hw-encoding-support/+merge/287176
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~morphis/aethercast/hw-encoding-support into lp:aethercast.



More information about the Ubuntu-reviews mailing list