[vlc-devel] [vlc-commits] demux: adaptative: fill result vectors by reference

[Hugo Beauzée-Luyssen]

On 10/06/2015 18:58, Francois Cartegnie wrote:
> vlc | branch: master | Francois Cartegnie <fcvlcdev at free.fr> | Sat Jun  6 14:10:08 2015 +0200| [fb4b0f0cea5579ae24ef3e5264a508ad78be2b14] | committer: Francois Cartegnie
>
> demux: adaptative: fill result vectors by reference
>
>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=fb4b0f0cea5579ae24ef3e5264a508ad78be2b14
> ---
>
>   .../adaptative/playlist/BaseRepresentation.cpp     |    3 +-
>   .../adaptative/playlist/SegmentInformation.cpp     |   31 ++++++++++----------
>   .../adaptative/playlist/SegmentInformation.hpp     |    4 +--
>   3 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/modules/demux/adaptative/playlist/BaseRepresentation.cpp b/modules/demux/adaptative/playlist/BaseRepresentation.cpp
> index 36ae73d..5561356 100644
> --- a/modules/demux/adaptative/playlist/BaseRepresentation.cpp
> +++ b/modules/demux/adaptative/playlist/BaseRepresentation.cpp
> @@ -59,7 +59,8 @@ void BaseRepresentation::debug(vlc_object_t *obj, int indent) const
>       std::string text(indent, ' ');
>       text.append("Representation");
>       msg_Dbg(obj, "%s", text.c_str());
> -    std::vector<ISegment *> list = getSegments();
> +    std::vector<ISegment *> list;
> +    getSegments(list);
>       std::vector<ISegment *>::const_iterator l;
>       for(l = list.begin(); l != list.end(); ++l)
>           (*l)->debug(obj, indent + 1);
> diff --git a/modules/demux/adaptative/playlist/SegmentInformation.cpp b/modules/demux/adaptative/playlist/SegmentInformation.cpp
> index 5eb5b6b..e63b6fc 100644
> --- a/modules/demux/adaptative/playlist/SegmentInformation.cpp
> +++ b/modules/demux/adaptative/playlist/SegmentInformation.cpp
> @@ -69,10 +69,8 @@ AbstractPlaylist * SegmentInformation::getPlaylist() const
>           return NULL;
>   }
>
> -vector<ISegment *> SegmentInformation::getSegments(SegmentInfoType type) const
> +std::size_t SegmentInformation::getSegments(SegmentInfoType type, vector<ISegment *> &retSegments) const
>   {
> -    vector<ISegment *> retSegments;
> -
>       switch (type)
>       {
>           case INFOTYPE_INIT:
> @@ -136,28 +134,28 @@ vector<ISegment *> SegmentInformation::getSegments(SegmentInfoType type) const
>       }
>
>       if( retSegments.empty() && parent )
> -        return parent->getSegments( type );
> +        return parent->getSegments( type, retSegments );
>       else
> -        return retSegments;
> +        return retSegments.size();
>   }
>
> -vector<ISegment *> SegmentInformation::getSegments() const
> +std::size_t SegmentInformation::getSegments(vector<ISegment *> &retSegments) const
>   {
> -    vector<ISegment *> retSegments;
>       for(int i=0; i<InfoTypeCount; i++)
>       {
> -        vector<ISegment *> segs = getSegments(static_cast<SegmentInfoType>(i));
> -        retSegments.insert( retSegments.end(), segs.begin(), segs.end() );
> +        vector<ISegment *> segs;
> +        if( getSegments(static_cast<SegmentInfoType>(i), segs) )
> +            retSegments.insert( retSegments.end(), segs.begin(), segs.end() );
>       }
> -    return retSegments;
> +    return retSegments.size();
>   }
>
>   ISegment * SegmentInformation::getSegment(SegmentInfoType type, uint64_t pos) const
>   {
>       ISegment *segment = NULL;
>
> -    vector<ISegment *> retSegments = getSegments( type );
> -    const size_t size = retSegments.size();
> +    vector<ISegment *> retSegments;
> +    const size_t size = getSegments( type, retSegments );
>       if( size )
>       {
>           /* check if that's a template (fixme: find a better way) */
> @@ -192,7 +190,8 @@ bool SegmentInformation::getSegmentNumberByTime(mtime_t time, uint64_t *ret) con
>       }
>       else
>       {
> -        const std::vector<ISegment *> segments = getSegments(INFOTYPE_MEDIA);
> +        std::vector<ISegment *> segments;
> +        getSegments(INFOTYPE_MEDIA, segments);
>           std::vector<ISegment *>::const_iterator it;
>           *ret = 0;
>           for(it = segments.begin(); it != segments.end(); ++it)
> @@ -254,7 +253,8 @@ void SegmentInformation::collectTimelines(std::vector<SegmentTimeline *> *timeli
>   void SegmentInformation::getDurationsRange(mtime_t *min, mtime_t *max) const
>   {
>       /* FIXME: cache stuff in segment holders */
> -    std::vector<ISegment *> seglist = getSegments(INFOTYPE_MEDIA);
> +    std::vector<ISegment *> seglist;
> +    getSegments(INFOTYPE_MEDIA, seglist);
>       std::vector<ISegment *>::const_iterator it;
>       mtime_t total = 0;
>       for(it = seglist.begin(); it != seglist.end(); ++it)
> @@ -344,7 +344,8 @@ static void insertIntoSegment(std::vector<ISegment *> &seglist, size_t start,
>
>   void SegmentInformation::SplitUsingIndex(std::vector<SplitPoint> &splitlist)
>   {
> -    std::vector<ISegment *> seglist = getSegments(INFOTYPE_MEDIA);
> +    std::vector<ISegment *> seglist;
> +    getSegments(INFOTYPE_MEDIA, seglist);
>       std::vector<SplitPoint>::const_iterator splitIt;
>       size_t start = 0, end = 0;
>       mtime_t time = 0;
> diff --git a/modules/demux/adaptative/playlist/SegmentInformation.hpp b/modules/demux/adaptative/playlist/SegmentInformation.hpp
> index ff43980..8dcf7cc 100644
> --- a/modules/demux/adaptative/playlist/SegmentInformation.hpp
> +++ b/modules/demux/adaptative/playlist/SegmentInformation.hpp
> @@ -82,8 +82,8 @@ namespace adaptative
>                   virtual void mergeWith(SegmentInformation *, mtime_t);
>
>               protected:
> -                std::vector<ISegment *> getSegments() const;
> -                std::vector<ISegment *> getSegments(SegmentInfoType) const;
> +                std::size_t getSegments(std::vector<ISegment *> &) const;
> +                std::size_t getSegments(SegmentInfoType, std::vector<ISegment *>&) const;
>                   std::vector<SegmentInformation *> childs;
>                   SegmentInformation *parent;
>
>
> _______________________________________________
> vlc-commits mailing list
> vlc-commits at videolan.org
> https://mailman.videolan.org/listinfo/vlc-commits
>

Hi,

I don't think this is required, since C++11 is enabled when available. 
Most compilers (at least those we use when building released binaries) 
will leverage move construction.
I also tend to think the return by value is also more readable.

Regards,

-- 
Hugo Beauzée-Luyssen