[Merge] lp:~phablet-team/media-hub/media-hub-desktop into lp:media-hub

Jim Hodapp jim.hodapp at canonical.com
Fri Aug 12 12:16:43 UTC 2016


Thanks for the very timely reviews guys. See my replies inline.

Diff comments:

> 
> === added file 'src/core/media/backend.cpp'
> --- src/core/media/backend.cpp	1970-01-01 00:00:00 +0000
> +++ src/core/media/backend.cpp	2016-08-11 19:37:36 +0000
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright (C) 2016 Canonical Ltd
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + *      http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + *
> + * Authored by: Jim Hodapp <jim.hodapp at canonical.com>
> + */
> +
> +#include "backend.h"
> +#include "core/media/logger/logger.h"
> +
> +#include <core/posix/signal.h>
> +
> +#include <cstring>
> +
> +namespace media = core::ubuntu::media;
> +
> +media::AVBackend::Backend media::AVBackend::get_backend_type()
> +{
> +    const char *backend = ::getenv("MH_BACKEND");

@Alfonso: While I do have a plan for using this env var for each platform type, I hadn't actually thought about detecting the presence of the GStreamer Android codecs. I was originally experimenting with detecting the presence of Android itself but that didn't work out because it's a build dep on every platform and modifying the debian package was going to be more complex and error-proned than using the env var. Let me think on this a bit.

@Konrad: I thought about doing that and initially I even began to do that, but then I thought there is other code in GStreamer that uses env vars and the environment var is a cache itself, so why take up more memory to double cache it? This also prevents me from doing something silly like using a singleton or externing a static variable. While it's not perfect, I like this approach the best.

> +    if (backend)
> +    {
> +        MH_DEBUG("MH_BACKEND: %s", backend);
> +        if (strcmp(backend, "hybris") == 0)
> +            return media::AVBackend::Backend::hybris;
> +        else
> +            return media::AVBackend::Backend::none;
> +    }
> +    else
> +    {
> +        return media::AVBackend::Backend::none;
> +    }
> +}


-- 
https://code.launchpad.net/~phablet-team/media-hub/media-hub-desktop/+merge/302712
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.



More information about the Ubuntu-reviews mailing list